So it looks like TransactionFeatureHandler has successfully taken over the
position of 'most in need of replacement'. Good old GMLBuilder has
relinquished its spot, as it has finally been replaced by the the geotools
gml producer.
As you've pointed out, TransactionFeatureHandler is a hacked up class that
makes a ton of assumptions. It's actually a forked version of geotools's
GMLFilterFeature, which made some even worse assumptions (features had to
have Member in their name, and the presence of an fid attribute meant it
was the typename). The TransactionFeatureHandler makes use of the fact
that GeoServer knows the names and namespaces of the features it is
expecting.
GMLFilterFeature, and by extension TransactionFeatureHandler, has suffered
neglect as GeoTools is planning on rewriting gml reading, to do it right,
reading in the schemas, and not making assumptions. So improving code
that is going to probably be thrown out seems less than ideal.
So in ideal world geotools motivates soon to write a real gml reader, and
GeoServer can just use it. Ian Schneider has some good ideas on how to go
about this, but can't do it himself, and is busy with other stuff for his
job. I've volunteered to help out when he has more time, and we're
looking for other people who are interested.
In our less than ideal world, it certainly is going to be at _least_ a
month until we get a good gml reader, and likely many months. So it might
make sense to try to improve TransactionFeatureHandler as best we can. It
looks as if it certainly has problems when working with Oracle.
TransactionFeatureHandler assumes that all properties are present. If
one is missing it fails to create correct features. By the way,
TransactionFeatureHandler creates new FeatureTypes based on the incoming
Feature!! Further if a property is present but does not have a value,
for instance because it is qualified as nil, it will not be present in
the Feature object.
Yeah, that was a hack that I forgot to document well. It works with
postgis, but that's because postgisDataSource is less stringent on
inserts. It will attempt to insert any feature that it recieves, relying
on the database to complain if it can't handle it. It just creates a
statement based on the attributes and types of the feature, so the
abbreviated features created by TransactionFeatureHandler work fine.
I made some quick and dirty fixes in TransactionFeatureHandler to allow
optional properties to be missing in a insert request.
I'd be more than happy to roll these changes if you'd like. It would be
nice if oracle could also allow optional properties, and then
PostgisDataStore in turn should probably be improved to do similar
typechecks to OracleDaatStore.
At the moment, everything is working smoothly with my modifications, so
I’m happy for now. Just thought, this would be things to consider in the
revised gml reader stuff.
Cool, thanks.
Something else I’ve stumbled across in TransactionFeatureHandler. It
reads xml attributes on gml features and adds them as attributes to the
Feature object. I don’t think this is the desired behavior. We are
creating the schema in DescribeFeatureType so we are completely in
control of the valid xml structure. I’d propose to only use subelements
for feature properties (not xml attributes).
Yeah, I've thought about this too. It's how things worked when I forked
from GMLFilterFeature, but I never took the time to really investigate
removing it. If you can make a wish in jira, and I'll get around to
investigating and implementing eventually.
Chris