wfs provider: remove old featureById and nextFeature

gml parsing: don't mistake attributes for features just because the have the same name and some cleanups

Author: jef-n

src/core/qgsgml.cpp

@@ -43,6 +43,7 @@ QgsGml::QgsGml(
     , mFinished( false )
     , mFeatureCount( 0 )
     , mCurrentWKBSize( 0 )
+    , mCurrentFeature( 0 )
 {
   mThematicAttributes.clear();
   for ( int i = 0; i < fields.size(); i++ )
@@ -140,7 +141,6 @@ int QgsGml::getFeatures( const QString& uri, QGis::WkbType* wkbType, QgsRectangl
 
 int QgsGml::getFeatures( const QByteArray &data, QGis::WkbType* wkbType, QgsRectangle* extent )
 {
-  QgsDebugMsg( "Entered" );
   mWkbType = wkbType;
   mExtent.setMinimal();
 
@@ -176,9 +176,11 @@ void QgsGml::handleProgressEvent( qint64 progress, qint64 totalSteps )
 void QgsGml::startElement( const XML_Char* el, const XML_Char** attr )
 {
   QString elementName( el );
+  ParseMode theParseMode( mParseModeStack.isEmpty() ? none : mParseModeStack.top() );
   QStringList splitName =  elementName.split( NS_SEPARATOR );
   QString localName = splitName.last();
   QString ns = splitName.size() > 1 ? splitName.first() : "";
+
   if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "coordinates" )
   {
     mParseModeStack.push( QgsGml::coordinate );
@@ -203,16 +205,17 @@ void QgsGml::startElement( const XML_Char* el, const XML_Char** attr )
   {
     mParseModeStack.push( QgsGml::boundingBox );
   }
-  else if ( localName == mTypeName )
+  else if ( theParseMode == none && localName == mTypeName )
   {
+    Q_ASSERT( !mCurrentFeature );
     mCurrentFeature = new QgsFeature( mFeatureCount );
     QgsAttributes attributes( mThematicAttributes.size() ); //add empty attributes
     mCurrentFeature->setAttributes( attributes );
     mParseModeStack.push( QgsGml::feature );
     mCurrentFeatureId = readAttribute( "fid", attr );
   }
 
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "Box" && mParseModeStack.top() == QgsGml::boundingBox )
+  else if ( theParseMode == boundingBox && elementName == GML_NAMESPACE + NS_SEPARATOR + "Box" )
   {
     //read attribute srsName="EPSG:26910"
     int epsgNr;
@@ -244,7 +247,7 @@ void QgsGml::startElement( const XML_Char* el, const XML_Char** attr )
   {
     mParseModeStack.push( QgsGml::multiPolygon );
   }
-  else if (( mParseModeStack.size() > 0 ) && ( mParseModeStack.top() == QgsGml::feature ) && ( mThematicAttributes.find( localName ) != mThematicAttributes.end() ) )
+  else if ( theParseMode == feature && mThematicAttributes.contains( localName ) )
   {
     mParseModeStack.push( QgsGml::attribute );
     mAttributeName = localName;
@@ -255,22 +258,18 @@ void QgsGml::startElement( const XML_Char* el, const XML_Char** attr )
 void QgsGml::endElement( const XML_Char* el )
 {
   QString elementName( el );
+  ParseMode theParseMode( mParseModeStack.isEmpty() ? none : mParseModeStack.top() );
   QStringList splitName =  elementName.split( NS_SEPARATOR );
   QString localName = splitName.last();
   QString ns = splitName.size() > 1 ? splitName.first() : "";
-  if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "coordinates" )
+
+  if ( theParseMode == coordinate && elementName == GML_NAMESPACE + NS_SEPARATOR + "coordinates" )
   {
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
   }
-  else if ( localName == mAttributeName ) //add a thematic attribute to the feature
+  else if ( theParseMode == attribute && localName == mAttributeName ) //add a thematic attribute to the feature
   {
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
 
     //find index with attribute name
     QMap<QString, QPair<int, QgsField> >::const_iterator att_it = mThematicAttributes.find( mAttributeName );
@@ -292,31 +291,27 @@ void QgsGml::endElement( const XML_Char* el )
           var = QVariant( mStringCash );
           break;
       }
+      Q_ASSERT( mCurrentFeature );
       mCurrentFeature->setAttribute( att_it.value().first, QVariant( mStringCash ) );
     }
   }
-  else if ( localName == mGeometryAttribute )
+  else if ( theParseMode == geometry && localName == mGeometryAttribute )
   {
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
   }
-  else if ( !mParseModeStack.empty() && mParseModeStack.top() == QgsGml::boundingBox && elementName == GML_NAMESPACE + NS_SEPARATOR + "boundedBy" )
+  else if ( theParseMode == boundingBox && elementName == GML_NAMESPACE + NS_SEPARATOR + "boundedBy" )
   {
     //create bounding box from mStringCash
     if ( createBBoxFromCoordinateString( mCurrentExtent, mStringCash ) != 0 )
     {
       QgsDebugMsg( "creation of bounding box failed" );
     }
 
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
   }
-  else if ( localName == mTypeName )
+  else if ( theParseMode == feature && localName == mTypeName )
   {
+    Q_ASSERT( mCurrentFeature );
     if ( mCurrentWKBSize > 0 )
     {
       mCurrentFeature->setGeometryAndOwnership( mCurrentWKB, mCurrentWKBSize );
@@ -336,11 +331,9 @@ void QgsGml::endElement( const XML_Char* el )
     {
       mIdMap.insert( mCurrentFeature->id(), mCurrentFeatureId );
     }
+    mCurrentFeature = 0;
     ++mFeatureCount;
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
   }
   else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "Point" )
   {
@@ -350,7 +343,7 @@ void QgsGml::endElement( const XML_Char* el )
       //error
     }
 
-    if ( mParseModeStack.top() != QgsGml::multiPoint )
+    if ( theParseMode == QgsGml::geometry )
     {
       //directly add WKB point to the feature
       if ( getPointWKB( &mCurrentWKB, &mCurrentWKBSize, *( pointList.begin() ) ) != 0 )
@@ -382,11 +375,6 @@ void QgsGml::endElement( const XML_Char* el )
       {
         QgsDebugMsg( "No wkb fragments" );
       }
-
-      //wkbList.push_back(wkb);
-      //wkbSizeList.push_back(wkbSize);
-      //mCurrentWKBFragments.push_back(wkbList);
-      //mCurrentWKBFragmentSizes.push_back(wkbSizeList);
     }
   }
   else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "LineString" )
@@ -398,7 +386,7 @@ void QgsGml::endElement( const XML_Char* el )
     {
       //error
     }
-    if ( mParseModeStack.top() != QgsGml::multiLine )
+    if ( theParseMode == QgsGml::geometry )
     {
       if ( getLineWKB( &mCurrentWKB, &mCurrentWKBSize, pointList ) != 0 )
       {
@@ -429,13 +417,9 @@ void QgsGml::endElement( const XML_Char* el )
       {
         QgsDebugMsg( "no wkb fragments" );
       }
-      //wkbList.push_back(wkb);
-      //wkbSizeList.push_back(wkbSize);
-      //mCurrentWKBFragments.push_back(wkbList);
-      //mCurrentWKBFragmentSizes.push_back(wkbSizeList);
     }
   }
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "LinearRing" )
+  else if (( theParseMode == geometry || theParseMode == multiPolygon ) && elementName == GML_NAMESPACE + NS_SEPARATOR + "LinearRing" )
   {
     QList<QgsPoint> pointList;
     if ( pointsFromCoordinateString( pointList, mStringCash ) != 0 )
@@ -458,42 +442,34 @@ void QgsGml::endElement( const XML_Char* el )
       QgsDebugMsg( "no wkb fragments" );
     }
   }
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "Polygon" )
+  else if (( theParseMode == geometry || theParseMode == multiPolygon ) && elementName == GML_NAMESPACE + NS_SEPARATOR + "Polygon" )
   {
     if ( *mWkbType != QGis::WKBMultiPolygon )//keep multitype in case of geometry type mix
     {
       *mWkbType = QGis::WKBPolygon;
     }
-    if ( mParseModeStack.top() != QgsGml::multiPolygon )
+
+    if ( theParseMode == geometry )
     {
       createPolygonFromFragments();
     }
   }
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiPoint" )
+  else if ( theParseMode == multiPoint && elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiPoint" )
   {
     *mWkbType = QGis::WKBMultiPoint;
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
     createMultiPointFromFragments();
   }
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiLineString" )
+  else if ( theParseMode == multiLine && elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiLineString" )
   {
     *mWkbType = QGis::WKBMultiLineString;
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
     createMultiLineFromFragments();
   }
-  else if ( elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiPolygon" )
+  else if ( theParseMode == multiPolygon && elementName == GML_NAMESPACE + NS_SEPARATOR + "MultiPolygon" )
   {
     *mWkbType = QGis::WKBMultiPolygon;
-    if ( !mParseModeStack.empty() )
-    {
-      mParseModeStack.pop();
-    }
+    mParseModeStack.pop();
     createMultiPolygonFromFragments();
   }
 }

src/core/qgsgml.h

@@ -38,7 +38,7 @@ class QgsCoordinateReferenceSystem;
  * uses the expat XML parser and an event based model to keep performance high.
  * The parsing starts when the first data arrives, it does not wait until the
  * request is finished */
-class CORE_EXPORT QgsGml: public QObject
+class CORE_EXPORT QgsGml : public QObject
 {
     Q_OBJECT
   public:
@@ -85,14 +85,10 @@ class CORE_EXPORT QgsGml: public QObject
     {
       none,
       boundingBox,
-      //featureMember, // gml:featureMember
-      feature,  // feature element containint attrs and geo (inside gml:featureMember)
+      feature,  // feature element containing attrs and geo (inside gml:featureMember)
       attribute,
       geometry,
       coordinate,
-      point,
-      line,
-      polygon,
       multiPoint,
       multiLine,
       multiPolygon

src/providers/wfs/qgswfsprovider.cpp

@@ -192,67 +192,6 @@ void QgsWFSProvider::copyFeature( QgsFeature* f, QgsFeature& feature, bool fetch
   feature.setFields( &mFields ); // allow name-based attribute lookups
 }
 
-bool QgsWFSProvider::featureAtId( QgsFeatureId featureId,
-                                  QgsFeature& feature,
-                                  bool fetchGeometry,
-                                  QgsAttributeList fetchAttributes )
-{
-  QMap<QgsFeatureId, QgsFeature* >::iterator it = mFeatures.find( featureId );
-  if ( it == mFeatures.end() )
-  {
-    return false;
-  }
-
-  QgsFeature* f = it.value();
-  if ( !f )
-  {
-    return false;
-  }
-
-  copyFeature( f, feature, fetchGeometry, fetchAttributes );
-  return true;
-}
-
-bool QgsWFSProvider::nextFeature( QgsFeature& feature )
-{
-  feature.setValid( false );
-
-  while ( true ) //go through the loop until we find a feature in the filter
-  {
-    if ( mSelectedFeatures.size() == 0 || mFeatureIterator == mSelectedFeatures.end() )
-    {
-      return 0;
-    }
-
-    QgsFeature* f = mFeatures[*mFeatureIterator];
-    ++mFeatureIterator;
-    if ( !f )
-    {
-      continue;
-    }
-
-    copyFeature( f, feature, mFetchGeom, mAttributesToFetch );
-
-    if ( mUseIntersect )
-    {
-      if ( feature.geometry() && feature.geometry()->intersects( mSpatialFilter ) )
-      {
-        return true;
-      }
-      else
-      {
-        continue; //go for the next feature
-      }
-    }
-    else
-    {
-      return true;
-    }
-  }
-}
-
-
-
 QGis::WkbType QgsWFSProvider::geometryType() const
 {
   return mWKBType;

src/providers/wfs/qgswfsprovider.h

@@ -49,29 +49,6 @@ class QgsWFSProvider: public QgsVectorDataProvider
 
     QgsFeatureIterator getFeatures( const QgsFeatureRequest& request = QgsFeatureRequest() );
 
-    /**
-     * Gets the feature at the given feature ID.
-     * @param featureId of the feature to be returned
-     * @param feature which will receive the data
-     * @param fetchGeometry flag which if true, will cause the geometry to be fetched from the provider
-     * @param fetchAttributes a list containing the indexes of the attribute fields to copy
-     * @return True when feature was found, otherwise false
-     *
-     * Default implementation traverses all features until it finds the one with correct ID.
-     * In case the provider supports reading the feature directly, override this function.
-     */
-    virtual bool featureAtId( QgsFeatureId featureId,
-                              QgsFeature& feature,
-                              bool fetchGeometry = true,
-                              QgsAttributeList fetchAttributes = QgsAttributeList() );
-
-    /**
-     * Get the next feature resulting from a select operation.
-     * @param feature feature which will receive data from the provider
-     * @return true when there was a feature to fetch, false when end was hit
-     */
-    virtual bool nextFeature( QgsFeature& feature );
-
     QGis::WkbType geometryType() const;
     long featureCount() const;