Jody,
this defect has the potential to cause some nasty, nasty problems. The
patch attached to the issue fixes these (in my opinion) and includes
test cases. These problems have always existed in SimpleFeatureTypeImpl,
and are only being exposed through the DAFFT API. I shudder every time I
port DSSFSFT code to DAFFT (a la GSIP 31) and iterate over properties. I
know that something will break if the patch is not applied.
It would be great if you could take a look at this. Help me sleep at
night. :-)
Kind regards,
Ben.
-------- Original Message --------
Subject: [jira] Updated: (GEOT-2338) SimpleFeatureTypeImpl has
inconsistent iteration order, broken equals/hashCode
Date: Thu, 19 Feb 2009 11:36:19 +0900
From: Ben Caradoc-Davies (JIRA) <j...@codehaus.org>
To: Caradoc-Davies, Ben (E&M, Kensington) <ben.caradoc-dav...@csiro.au>
[
http://jira.codehaus.org/browse/GEOT-2338?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ben Caradoc-Davies updated GEOT-2338:
-------------------------------------
Priority: Critical (was: Major)
This will cause severe problems when people start using GeoServer with
GSIP 31 support. Unless you want your simple feature properties randomly
reordered, that is.
> SimpleFeatureTypeImpl has inconsistent iteration order, broken equals/hashCode
> ------------------------------------------------------------------------------
>
> Key: GEOT-2338
> URL: http://jira.codehaus.org/browse/GEOT-2338
> Project: GeoTools
> Issue Type: Bug
> Components: core main
> Affects Versions: 2.6-M2
> Reporter: Ben Caradoc-Davies
> Assignee: Jody Garnett
> Priority: Critical
> Attachments: SimpleFeatureTypeImpl.updated.patch
>
>
> SimpleFeatureTypeImpl returns its internal property descriptor list via
> getAttributeDescriptors(), but when accessed via the FeatureType API
> getDescriptors(), will return its property descriptors in a different order.
> ComplexTypeImpl (grandparent of SimpleFeatureTypeImpl) uses a HashMap to
> store descriptors, so getDescriptors(), which iterates over this Map, will
> return descriptors in a random order. This is *A Bad Thing*, because
> descriptor order is *very* significant for simple feature types. Furthermore,
> ComplexTypeImpl uses Map semantics for equals/hashCode, which is guaranteed
> to be order independent. This means that two SimpleFeatureTypeImpl instances
> with the same properties in a different order must be equal. This is also *A
> Very Bad Thing*.
> Changing ComplexTypeImpl to use LinkedHashMap to fix the the iteration order
> will not fix the equals/hashCode problem. It is more straightforward and
> likely faster to override getDescriptors() to return the descriptors list.
> Likewise, equals and hashCode are easily fixed.
> The attached patch:
> (1) Fixes iteration order by ensuring getAttributeDescriptors() and
> getAttributeDescriptors() return the same immutable object.
> (2) Makes these methods final to preserve the contract.
> (3) Ensures the descriptors member is immutable by making it an unmodifiable
> copy.
> (4) Adds unit tests for iteration order and equality.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
--
Ben Caradoc-Davies <ben.caradoc-dav...@csiro.au>
Software Engineer, CSIRO Exploration and Mining
Australian Resources Research Centre
26 Dick Perry Ave, Kensington WA 6151, Australia
------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
Geotools-devel mailing list
Geotools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
