Justin Deoliveira ha scritto:
> There should be two options for creating a new feature type via the REST 
> api:
> 
> 1) POST /rest/workspaces/<ws>/datastores/<store>/featureTypes
> 
> Where the content must contain the name of the feature type, along with 
> all the other information about attributes
> 
> or:
> 
> 2) PUT to /rest/workspaces/<ws>/datastores/<store>/featureTypes/<ftName>
> 
> Where the content contains optionally the feature type name, along with 
> all the other information about attributes

Ok, so I need to implement the PUT one.
A question, is it normal that the server mangles the document that
you're going to PUT in a certain location?
As far as I can see the REST Config extension allows PUT only to
over resources that already exist, all the handlePut methods I've
seen end up with catalog.save(xxx) instead of catalog.add(xxx).
Why the exception for feature types?

> Looked over the patch and a few comments:
> 
> * When throwing exceptions it is best to throw a RestletException and 
> pass in a status code, else the client will just receive a 500 (server 
> internal error). But for the validation cases in 
> FeautreTypeResource.buildFeatureType() it is more appropriate to send 
> back an error relaying that the client did not specify the request 
> properly. Probably a 400 or something.

Aah, very good, I was wondering why there was no declared exception
in the various handleXYZ methods.

> * Rename XStreamPersister.setHideAttributes() to something more 
> descriptive of being applicable only to feature types, 
> setHideFeatureTypeAttributes() or something.

Cool

> * I see some dead code in the patch for XStreamPersister? doUnmarshal()

Right, leftovers of an experiment I did, I'll remove them.

> * The change to ResourcePool:
> 
> -        if (info.getAttributes() != null && 
> !info.getAttributes().isEmpty()) {
> +        if (info.getAttributes() != null && 
> !info.getAttributes().isEmpty() && 
> info.getAttributes().get(0).getBinding() != null) {
> 
> Can you explain the rationale here?

If the binding is not available in the first attribute it means
the attributes are old and miss some information (binding, length)
so they should be reloaded (otherwise attributes() won't report
complete information).

> Also a heads up. One of the pains of xstream is adding new properties to 
> an object but also dealing with old representations of it on disk. Since 
> xstream does not use the default constructor any newly defined 
> attributes that are not in a representation will always be set to null 
> regardless of what the default constructor does.
> 
> So in this case when reading older configurations 
> AttributeTypeINfo.getBInding() and getLength() will result in null. 
> Perhaps this is ok... but it might be wise to implement 
> AttributeTypeInfoImpl.readResolve() to initialize them to sensible defaults.

It's not like we have a good reasonable default. The sensible action
would be to go grab the feature types and fill binding and length
with the real values. Or leave them null so that the
code above fills them with the real values on demand.

Cheers
Andrea

-- 
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to