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)
* 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).
* 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
-------------------------------------------------------------------------
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
