On Monday 31 March 2008 11:44:07 Andrea Aime wrote:
> Hi,
> since I'm using the rest module for the first time, I thought I
> could use the occasion to provide a little feedback.
>
> Here we go (long one, grab seat, coffee, muffins and whatnot):
> * I've noticed the BeanResourceFinder is declared both in
> rest and RESTConfig (seems a copy and paste)
Right, the rest module was basically pulled from RESTConfig. The copy of
RESTConfig that lives in the 1.6.x branch shouldn't be taken too seriously,
the trunk version is the one I'm actually working on. The 1.6.x version is a
somewhat older version that I just commented out parts of to have something
that compiled against the rest module.
> * spring configuration seems a little verbose. Instead of
> declaring the path in the map, can't we have the resources
> themselves provide the path, and just declare the resources
> in the spring context? Something like:
> <bean name="layerGroupResource"
> class="org.geoserver.restconfig.LayerGroupListResource">
> <property name="WMSConfig">
> <ref bean="wmsConfig" />
> </property>
> <property name="location" value="/layergroups/{group}.{type}"/>
> </bean>
> </property>
> </bean>
> This of course would be an extra, that is, it should not
> disallow people from writing their own restlets of finders
> (maybe the default router could scan the context for custom restlet,
> finders and resources that do expose a location property,
> which means, custom geoserver subclasses of the ones provided
> by Restlet).
The thought behind doing it this way is that you may want to have different
paths to the same resource (for a trivial example, you might want to allow
/foo and /foo?{query} and have them handled by the same restlet).
> * MapResource should probably throw an Exception in the putMap
> method, if the subclass does not override it a better error is, imho,
> better than silence
> * MapResources does not implement handlePost, yet it would be probably
> handy to have one for the cases where using POST is justified
> (resource creation whose url cannot be computed by the client)
> * I do not see any unit or functional testing in either rest
> and RestConfig... /me cries...
> * MapResource and DataFormat. I see lots of places where the same
> types are repeated, like "json"... can't DataFormat have a
> getType() method, and we use it directly (so we turn that
> getSupportedFormats return type from Map to List)?
> * Json wise, it seems quite common to return not only json
> maps, but also arrays. One could roll a ListResource, but
> also DataFormats assumes a map.
> What about having a CollectionResource instead? It's a bit
> more generic...
> * I have the strong impression the whole rest thing, as implemented,
> is not thread safe. You instantiate the bean finder as a singleton,
> that references a singleton resorce. When the call is made,
> the context, request and response is provided using
> myBeanToFind.init(getContext(), request, response);
> Now, what happens when two threads do hit the same url with
> different params at the same time? Boom :(
> * MapResource.getMap() cannot throw any exception? How do I
> tell the world that something went wrong? Not sure if it's
> better to let it throw a generic exception like putMap(...)
> or have something that's rest specific.
> For example, how would you distinguish between a security
> issue and an internal error? Could be done with generic
> exceptions, provided an exception translator could be found
> to turn a generic exception to an HTTP error code
>
> Ok, I'll write further observations as I find them down the road,
> enough for the first mail
> Cheers
> Andrea
Thanks for the comments, Andrea.
> !DSPAM:4040,47f106d2201412085621377!
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
