[Geoserver-devel] Creation of a new table/shapefile by POST, first steps

Andrea_Aime3 · March 23, 2010, 5:04pm

Hi,
I have a first patch for the above attached to
http://jira.codehaus.org/browse/GEOS-3872

I think I need a clarification though. I though we were
going to post to a feature type description to:
/rest/workspaces/<ws>/datastores/<store>

but as far as I can see DataStoreResource already handles
post and it does for the creation of a new store... I guess
the stores list delegates to it for the post handling somehow.

So I added the creation code into FeatureTypeResource.
The latter was already managing POST with the assumption
the feature type was already in the store and just needed
to be published, with the patch it will first look to see
if it's there, if not, it will create it and then
configure it.

I see how this can be troublesome though, a typo in the
feature type name post-ed and an attempted configuration
will turn into the creation of a new feature type
(well, sort of, it will fail anyways you did not specify any
attribute list).

However, if it's considered more desirable to post directly
to the store resources, what other changes are necessary?

Cheers
Andrea

PS: I know the previous mails talked about a bigger
change, but since this is low priority work that is
done only when I have nothing else to do I'd prefer
to start commiting smaller, self contained patches that
add one bit of the overall plan at a time.

Justin_Deoliveira · March 24, 2010, 12:39pm

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

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.

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

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

* 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?

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.

Nice work on the patch. A very cool feature.

-Justin

On 3/23/10 1:04 PM, Andrea Aime wrote:

Hi,
I have a first patch for the above attached to
http://jira.codehaus.org/browse/GEOS-3872

I think I need a clarification though. I though we were
going to post to a feature type description to:
/rest/workspaces/<ws>/datastores/<store>

but as far as I can see DataStoreResource already handles
post and it does for the creation of a new store... I guess
the stores list delegates to it for the post handling somehow.

So I added the creation code into FeatureTypeResource.
The latter was already managing POST with the assumption
the feature type was already in the store and just needed
to be published, with the patch it will first look to see
if it's there, if not, it will create it and then
configure it.

I see how this can be troublesome though, a typo in the
feature type name post-ed and an attempted configuration
will turn into the creation of a new feature type
(well, sort of, it will fail anyways you did not specify any
attribute list).

However, if it's considered more desirable to post directly
to the store resources, what other changes are necessary?

Cheers
Andrea

PS: I know the previous mails talked about a bigger
change, but since this is low priority work that is
done only when I have nothing else to do I'd prefer
to start commiting smaller, self contained patches that
add one bit of the overall plan at a time.

------------------------------------------------------------------------------
Download Intel® 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
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Andrea_Aime3 · March 24, 2010, 1:46pm

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.

Justin_Deoliveira · March 25, 2010, 1:47pm

On 3/24/10 9:46 AM, Andrea Aime wrote:

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?

Yeah, if we follow the convention already there then we can use POST just for modify. Just noted it because the http semantics of POST allow one to POST to a collection and have a new resource created. Whereas PUT only allows you to PUT directly to a resource, whether it exists already or does not.

But yeah, I am all for having POST to create a new feature type and PUT to modify an existing one.

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

Ok, makes sense. Can you add a comment there describing the rationale.

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.

Works for me.

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.