GeoTools
  1. GeoTools
  2. GEOT-3760

SimpleFeatureImpl misses getName(), getDescriptor(), and returning GeometryAttribute for geometry properties to comply to the Feature contract

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.7.2
    • Fix Version/s: 2.7.3
    • Component/s: main
    • Testcase included:
      yes

      Description

      SimpleFeatureImpl doesn't comply to the Feature contract. It misses getName(), getDescriptor(), and returning an instance of GeometryAttribute for geometry properties, making it not suitable to be used by code that works on plain Features

      Patch and test case:

      diff --git a/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureImpl.java b/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureImpl.java
      index 179ff8f..0514718 100644
      --- a/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureImpl.java
      +++ b/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureImpl.java
      @@ -29,6 +29,7 @@ import org.geotools.feature.AttributeImpl;
       import org.geotools.feature.GeometryAttributeImpl;
       import org.geotools.feature.IllegalAttributeException;
       import org.geotools.feature.PropertyImpl;
      +import org.geotools.feature.type.AttributeDescriptorImpl;
       import org.geotools.feature.type.Types;
       import org.geotools.geometry.jts.ReferencedEnvelope;
       import org.geotools.util.Converters;
      @@ -40,6 +41,7 @@ import org.opengis.feature.simple.SimpleFeatureType;
       import org.opengis.feature.type.AttributeDescriptor;
       import org.opengis.feature.type.AttributeType;
       import org.opengis.feature.type.GeometryDescriptor;
      +import org.opengis.feature.type.GeometryType;
       import org.opengis.feature.type.Name;
       import org.opengis.filter.identity.FeatureId;
       import org.opengis.filter.identity.Identifier;
      @@ -355,12 +357,20 @@ public class SimpleFeatureImpl implements SimpleFeature {
               setValue( (Collection<Property>) newValue );
           }
           
      +    /**
      +     * @see org.opengis.feature.Attribute#getDescriptor()
      +     */
           public AttributeDescriptor getDescriptor() {
      -        return null;
      +        return new AttributeDescriptorImpl(featureType, featureType.getName(), 0,
      +                Integer.MAX_VALUE, true, null);
           }
       
      +    /**
      +     * @return same name than this feature's {@link SimpleFeatureType}
      +     * @see org.opengis.feature.Property#getName()
      +     */
           public Name getName() {
      -        return null;
      +        return featureType.getName();
           }
       
           public boolean isNillable() {
      @@ -450,8 +460,12 @@ public class SimpleFeatureImpl implements SimpleFeature {
            */
           class AttributeList extends AbstractList<Property> {
       
      -        public Attribute get(int index) {
      -            return new Attribute( index );
      +        public Property get(int index) {
      +            AttributeDescriptor descriptor = featureType.getDescriptor(index);
      +            if (descriptor instanceof GeometryDescriptor) {
      +                return new SimpleGeometryAttribute(index);
      +            }
      +            return new Attribute(index);
               }
               
               public Attribute set(int index, Property element) {
      @@ -580,5 +594,54 @@ public class SimpleFeatureImpl implements SimpleFeature {
               }
           }
           
      -    
      +    class SimpleGeometryAttribute extends Attribute implements GeometryAttribute {
      +
      +        SimpleGeometryAttribute(int index) {
      +            super(index);
      +        }
      +
      +        @Override
      +        public GeometryType getType() {
      +            return (GeometryType) super.getType();
      +        }
      +
      +        @Override
      +        public GeometryDescriptor getDescriptor() {
      +            return (GeometryDescriptor) super.getDescriptor();
      +        }
      +
      +        @Override
      +        public BoundingBox getBounds() {
      +            ReferencedEnvelope bounds = new ReferencedEnvelope(
      +                    featureType.getCoordinateReferenceSystem());
      +            Object value = getAttribute(index);
      +            if (value instanceof Geometry) {
      +                bounds.init(((Geometry) value).getEnvelopeInternal());
      +            }
      +            return bounds;
      +        }
      +
      +        @Override
      +        public void setBounds(BoundingBox bounds) {
      +            // do nothing, this property is strictly derived. Shall throw unsupported operation
      +            // exception?
      +        }
      +
      +        @Override
      +        public int hashCode() {
      +            return 17 * super.hashCode();
      +        }
      +
      +        @Override
      +        public boolean equals(Object obj) {
      +            if (this == obj) {
      +                return true;
      +            }
      +
      +            if (!(obj instanceof SimpleGeometryAttribute)) {
      +                return false;
      +            }
      +            return super.equals(obj);
      +        }
      +    }
       }
      diff --git a/modules/library/main/src/test/java/org/geotools/feature/simple/SimpleFeatureImplTest.java b/modules/library/main/src/test/java/org/geotools/feature/simple/SimpleFeatureImplTest.java
      index 9d5b204..15c065d 100644
      --- a/modules/library/main/src/test/java/org/geotools/feature/simple/SimpleFeatureImplTest.java
      +++ b/modules/library/main/src/test/java/org/geotools/feature/simple/SimpleFeatureImplTest.java
      @@ -22,6 +22,7 @@ import org.geotools.data.DataUtilities;
       import org.opengis.feature.GeometryAttribute;
       import org.opengis.feature.simple.SimpleFeature;
       import org.opengis.feature.simple.SimpleFeatureType;
      +import org.opengis.feature.type.GeometryDescriptor;
       
       public class SimpleFeatureImplTest extends TestCase {
           
      @@ -56,7 +57,23 @@ public class SimpleFeatureImplTest extends TestCase {
           public void testDefaultGeometryProperty(){
               assertTrue("expected GeometryAttribute, got " + feature.getProperty("the_geom").getClass().getName(),
                       feature.getProperty("the_geom") instanceof GeometryAttribute);
      -        assertNotNull(feature.getDefaultGeometryProperty());
      -        assertNull(feature.getDefaultGeometryProperty().getValue());
      +        GeometryAttribute defaultGeometryProperty = feature.getDefaultGeometryProperty();
      +        assertNotNull(defaultGeometryProperty);
      +        assertNull(defaultGeometryProperty.getValue());
      +        assertNotNull(defaultGeometryProperty.getDescriptor());
      +        assertTrue(defaultGeometryProperty.getDescriptor() instanceof GeometryDescriptor);
      +    }
      +    
      +    public void testGetName(){
      +        assertNotNull(feature.getName());
      +        assertEquals(feature.getFeatureType().getName(), feature.getName());
      +    }
      +    
      +    public void testGetDescriptor() {
      +        assertNotNull(feature.getDescriptor());
      +        assertSame(feature.getType(), feature.getDescriptor().getType());
      +        assertTrue(feature.getDescriptor().isNillable());
      +        assertEquals(0, feature.getDescriptor().getMinOccurs());
      +        assertEquals(Integer.MAX_VALUE, feature.getDescriptor().getMaxOccurs());
           }
       }
      

        Activity

        Hide
        Andrea Aime added a comment -
        Looks good. One day it would be nice to have FeatureImpl have some respect for memory usage and performance as well, so that it can be used directly instead of needing this alternate implementation for dealing with non trivial amounts of data...
        Show
        Andrea Aime added a comment - Looks good. One day it would be nice to have FeatureImpl have some respect for memory usage and performance as well, so that it can be used directly instead of needing this alternate implementation for dealing with non trivial amounts of data...
        Hide
        Gabriel Roldan added a comment -
        fixed at r37737 for trunk and r37740 for 2.7.x
        Show
        Gabriel Roldan added a comment - fixed at r37737 for trunk and r37740 for 2.7.x

          People

          • Assignee:
            Gabriel Roldan
            Reporter:
            Gabriel Roldan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: