Second round of feedback from Andrea

modules/extension/xsd/xsd-wfs/src/main/java/org/geotools/wfs/bindings/WFSParsingUtils.java

@@ -29,6 +29,7 @@
 import net.opengis.wfs20.Wfs20Factory;
 
 import org.eclipse.emf.ecore.EObject;
+import org.geotools.data.DataUtilities;
 import org.geotools.data.simple.SimpleFeatureCollection;
 import org.geotools.feature.DefaultFeatureCollection;
 import org.geotools.feature.FeatureCollection;
@@ -56,25 +57,16 @@ public static EObject FeatureCollectionType_parse(EObject fct, ElementInstance i
         //check for an array
         SimpleFeature[] featureMembers = (SimpleFeature[]) node.getChildValue(SimpleFeature[].class);
         if (featureMembers != null) {
-            if( fc instanceof Collection){
-                for (int i = 0; i < featureMembers.length; i++) {
-                    ((Collection)fc).add(featureMembers[i]);
-                }
-            }
-            else {
-                throw new IllegalStateException("Require DefaultFeatureCollection or ListFeatureCollection");
+            Collection<SimpleFeature> collection = DataUtilities.collectionCast( fc );
+            for (int i = 0; i < featureMembers.length; i++) {
+                collection.add(featureMembers[i]);
             }
         }
         else {
-            //gml:featureMember
-            if( fc instanceof Collection){
-                List<SimpleFeature> featureMember = node.getChildValues( SimpleFeature.class );
-                for (SimpleFeature f : featureMember ) {
-                    ((Collection)fc).add( f );
-                }
-            }
-            else {
-                throw new IllegalStateException("Require DefaultFeatureCollection or ListFeatureCollection");
+            Collection<SimpleFeature> collection = DataUtilities.collectionCast( fc );
+            List<SimpleFeature> featureMember = node.getChildValues( SimpleFeature.class );
+            for (SimpleFeature f : featureMember ) {
+                collection.add( f );
             }
         }
 

modules/library/main/src/main/java/org/geotools/data/DataUtilities.java

@@ -1146,41 +1146,6 @@ public static SimpleFeatureSource createView(final DataStore store, final Query
             throws IOException, SchemaException {
         return new DefaultView(store.getFeatureSource(query.getTypeName()), query);
     }
-//    /**
-//     * Returns collection from the provided feature array
-//     * 
-//     * @param featureArray 
-//     * @return array contents as a SimpleFeatureCollection
-//     * @deprecated Please use {@link #collection(SimpleFeature[])} and check {@link SimpleFeatureCollection#size()} to ensure contents are not empty
-//     */
-//    public static SimpleFeatureCollection results(SimpleFeature[] featureArray) {
-//        return results(collection(featureArray));
-//    }
-
-//    /**
-//     * Returns collection if non empty.
-//     * <p>
-//     * Previous implementation would throw an IOException if the collection was empty; method
-//     * is now duplicated as it does not servce a function.
-//     * 
-//     * @param collection
-//     * @return provided collection
-//     * @deprecated Please check collection.size() directly to ensure your collection contains content
-//     */
-//    public static SimpleFeatureCollection results(final SimpleFeatureCollection collection) {
-//        if (collection.size() == 0) {
-//            // throw new IOException("Provided collection was empty");
-//        }
-//        return collection;
-//    }
-
-//    public static <T extends FeatureType, F extends Feature> FeatureCollection<T, F> results(
-//            final FeatureCollection<T, F> collection) {
-//        if (collection.size() == 0) {
-//            // throw new IOException("Provided collection was empty");
-//        }
-//        return collection;
-//    }
 
     /**
      * Adapt a collection to a reader for use with FeatureStore.setFeatures( reader ).
@@ -1267,37 +1232,13 @@ public static void close(Iterator<?> iterator) {
             }
         }
     }
-
-    /**
-     * Obtain the first feature from the collection as an exemplar.
-     * 
-     * @param simpleFeatureCollection
-     * @return first feature from the featureCollection
-     */
-    public static SimpleFeature first(SimpleFeatureCollection simpleFeatureCollection) {
-        if (simpleFeatureCollection == null) {
-            return null;
-        }
-        SimpleFeatureIterator iter = simpleFeatureCollection.features();
-        try {
-            while (iter.hasNext()) {
-                SimpleFeature feature = iter.next();
-                if (feature != null) {
-                    return feature;
-                }
-            }
-            return null; // not found!
-        } finally {
-            iter.close();
-        }
-    }
+
     /**
      * Obtain the first feature from the collection as an exemplar.
      * 
      * @param featureCollection
      * @return first feature from the featureCollection
      */
-
     public static <F extends Feature> F first( FeatureCollection<?,F> featureCollection ){
         if (featureCollection == null) {
             return null;

modules/library/main/src/main/java/org/geotools/feature/collection/BaseFeatureCollection.java

@@ -197,18 +197,21 @@ public void accepts(org.opengis.feature.FeatureVisitor visitor,
             float position = 0;
             progress.started();
             for (iterator = features(); !progress.isCanceled() && iterator.hasNext();) {
-                if (size > 0)
-                    progress.progress(position++ / size);
                 try {
                     SimpleFeature feature = (SimpleFeature) iterator.next();
                     visitor.visit(feature);
                 } catch (Exception erp) {
                     progress.exceptionOccurred(erp);
                 }
+                if (size > 0){
+                    progress.progress(position++ / size);
+                }
             }
         } finally {
             progress.complete();
-            iterator.close();
+            if( iterator != null ){
+                iterator.close();
+            }
         }
     }
 

modules/library/main/src/test/java/org/geotools/data/DataUtilitiesTest.java

@@ -110,7 +110,12 @@ public void testSimpleSource() {
         assertSame( simple, source);  
     } 
 
-
+    public void testFirst() {
+        SimpleFeatureCollection collection = DataUtilities.collection( roadFeatures );
+         SimpleFeature first = DataUtilities.first( collection );
+        assertNotNull( first);
+        assertEquals( "road.rd1", first.getID() );
+    }
     public void testCheckDirectory() {
         File home = new File(System.getProperty("user.home"));
         File file = DataUtilities.checkDirectory(home);

modules/library/main/src/test/java/org/geotools/feature/FeatureCollectionTest.java

@@ -55,14 +55,8 @@ public FeatureCollectionTest(String testName) {
         super(testName);
     }
     protected SimpleFeatureCollection newCollection(SimpleFeatureType schema, List<SimpleFeature> list ) {
-        SimpleFeatureCollection features = FeatureCollections.newCollection();
-        if( features instanceof Collection ){
-            Collection<SimpleFeature> collection = (Collection) features;
-            collection.addAll( list );
-            return features;
-        }
-        else {
-            throw new IllegalStateException("Please configure test wih provided contents");
-        }
+        DefaultFeatureCollection features = new DefaultFeatureCollection();
+        features.addAll( list );
+        return features;
     }
 }

modules/library/render/src/main/java/org/geotools/renderer/lite/StreamingRenderer.java

@@ -676,16 +676,15 @@ public void paint(Graphics2D graphics, Rectangle paintArea,
         // Check for null arguments, recompute missing ones if possible
         //
         // ////////////////////////////////////////////////////////////////////
-        if (graphics == null || paintArea == null) {
-            LOGGER.severe("renderer passed null arguments");
-            throw new NullPointerException("renderer passed null arguments");
-        } else if (mapArea == null && paintArea == null) {
-            LOGGER.severe("renderer passed null arguments");
-            throw new NullPointerException("renderer passed null arguments");
+        if (graphics == null) {
+            LOGGER.severe("renderer passed null graphics argument");
+            throw new NullPointerException("renderer requires graphics");
+        } else if (paintArea == null) {
+            LOGGER.severe("renderer passed null paintArea argument");
+            throw new NullPointerException("renderer requires paintArea");
         } else if (mapArea == null) {
-
-            LOGGER.severe("renderer passed null arguments");
-            throw new NullPointerException("renderer passed null arguments");
+            LOGGER.severe("renderer passed null mapArea argument");
+            throw new NullPointerException("renderer requires mapArea");
         } else if (worldToScreen == null) {
             worldToScreen = RendererUtilities.worldToScreenTransform(mapArea,
                     paintArea);
@@ -1977,7 +1976,7 @@ private void processStylers(final Graphics2D graphics,
                 Query query = getLayerQuery(currLayer, featureSource, schema,
                         uniform, mapArea, destinationCrs, sourceCrs, screenSize,
                         geometryAttribute, at);
-                FeatureCollection rawFeatures;
+                FeatureCollection<?,?> rawFeatures;
                 if(transform != null) {
                     GridEnvelope2D ge = new GridEnvelope2D(screenSize);
                     ReferencedEnvelope re = new ReferencedEnvelope(mapArea, destinationCrs);
@@ -2115,7 +2114,9 @@ FeatureCollection applyRenderingTransformation(Expression transformation,
                     // similar to this (like passing it as a param to readCoverage
                 }
 
-                Feature gridWrapper = featureSource.getFeatures().features().next();
+                FeatureCollection<?,?> sample = featureSource.getFeatures();
+                Feature gridWrapper = DataUtilities.first( sample );
+
                 if(FeatureUtilities.isWrappedCoverageReader(simpleSchema)) {
                     final Object params = paramsPropertyName.evaluate(gridWrapper);
                     final AbstractGridCoverage2DReader reader = (AbstractGridCoverage2DReader) gridPropertyName.evaluate(gridWrapper);
@@ -2484,46 +2485,31 @@ void rescaleFeatureTypeStyle(LiteFeatureTypeStyle fts, DuplicatingStyleVisitor v
      */
     private void drawPlain(final Graphics2D graphics, MapLayer currLayer, AffineTransform at,
             CoordinateReferenceSystem destinationCrs, String layerId, Collection collection,
-            FeatureCollection features, final NumberRange scaleRange, final List lfts) {
-        final LiteFeatureTypeStyle[] fts_array = (LiteFeatureTypeStyle[]) lfts
-        .toArray(new LiteFeatureTypeStyle[lfts.size()]);
+            FeatureCollection<?,?> features, final NumberRange scaleRange,
+            final List<LiteFeatureTypeStyle> lfts) {
+
+        final LiteFeatureTypeStyle[] fts_array = lfts.toArray(new LiteFeatureTypeStyle[lfts.size()]);
 
         // for each lite feature type style, scan the whole collection and draw
         for (LiteFeatureTypeStyle liteFeatureTypeStyle : fts_array) {
-
+            Iterator<?> iterator = null;
             if (collection != null){
-                Iterator iterator = collection.iterator();
+                iterator = collection.iterator();
                 if (iterator == null ){
                     return; // nothing to do
                 }
-                try {
-                    boolean clone = isCloningRequired(currLayer, fts_array);
-                    RenderableFeature rf = new RenderableFeature(currLayer, clone);
-                    // loop exit condition tested inside try catch
-                    // make sure we test hasNext() outside of the try/cath that follows, as that
-                    // one is there to make sure a single feature error does not ruin the rendering
-                    // (best effort) whilst an exception in hasNext() + ignoring catch results in
-                    // an infinite loop
-                    while (iterator.hasNext() && !renderingStopRequested) {
-                        try {
-                            rf.setFeature(iterator.next());
-                            process(rf, liteFeatureTypeStyle, scaleRange, at, destinationCrs, layerId);
-                        } catch (Throwable tr) {
-                            fireErrorEvent(tr);
-                        }
-                    }
-                } finally {
-                    DataUtilities.close( iterator );
-                }
             }
-
-
-            if( features != null ){
-                FeatureIterator<?> iterator = features.features();
-                if (iterator == null ){
+            else if (features != null ){
+                FeatureIterator<?> featureIterator = ((FeatureCollection<?,?>)features).features();
+                if( featureIterator == null ){
                     return; // nothing to do
-                }  
-                try {
+                }
+                iterator = DataUtilities.iterator( featureIterator );
+            }
+            else {
+                return; // nothing to do
+            }
+            try {
                     boolean clone = isCloningRequired(currLayer, fts_array);
                     RenderableFeature rf = new RenderableFeature(currLayer, clone);
                     // loop exit condition tested inside try catch
@@ -2539,9 +2525,8 @@ private void drawPlain(final Graphics2D graphics, MapLayer currLayer, AffineTran
                             fireErrorEvent(tr);
                         }
                     }
-                } finally {
-                    iterator.close();
-                }
+            } finally {
+                DataUtilities.close( iterator );
             }
         }
     }

modules/unsupported/geogit-versioned/src/test/java/org/geoserver/data/geogit/GeoGitFeatureStoreTest.java

@@ -194,20 +194,9 @@ public void testUseProvidedFIDSupported() throws Exception {
         }
     }
     private SimpleFeature sample( SimpleFeatureSource store, Filter filter ) throws IOException {
-        SimpleFeatureIterator iterator = null;
-        try {
-            SimpleFeatureCollection result = store.getFeatures( filter );
-            iterator = result.features();
-
-            return iterator.next();
-        }
-        finally {
-            if( iterator != null ){
-                iterator.close();
-            }
-        }
+        SimpleFeatureCollection result = store.getFeatures( filter );
+        return DataUtilities.first( result );
     }
-    @SuppressWarnings("deprecation")
     public void testModifyFeatures() throws Exception {
         // add features circunventing FeatureStore.addFeatures to keep the test
         // independent of the

modules/unsupported/process-feature/src/main/java/org/geotools/process/vector/ProcessingCollection.java

@@ -83,7 +83,7 @@ public void accepts(FeatureVisitor visitor, ProgressListener progress) throws IO
             float position = 0;
             progress.started();
             iterator = features();
-            while (iterator.hasNext()) {
+            while (!progress.isCanceled() && iterator.hasNext()) {
                 try {
                     F feature = iterator.next();
                     visitor.visit(feature);