diff --git a/src/providers/delimitedtext/qgsdelimitedtextfile.cpp b/src/providers/delimitedtext/qgsdelimitedtextfile.cpp index d5af5b0f7cc..e108335079f 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextfile.cpp +++ b/src/providers/delimitedtext/qgsdelimitedtextfile.cpp @@ -82,6 +82,11 @@ void QgsDelimitedTextFile::close() delete mWatcher; mWatcher = 0; } + mLineNumber = -1; + mRecordLineNumber = -1; + mRecordNumber = -1; + mMaxRecordNumber = -1; + mHoldCurrentRecord = false; } bool QgsDelimitedTextFile::open() @@ -95,25 +100,24 @@ bool QgsDelimitedTextFile::open() QgsDebugMsg( "Data file " + mFileName + " could not be opened" ); delete mFile; mFile = 0; - return false; } - mStream = new QTextStream( mFile ); - if ( ! mEncoding.isEmpty() ) + if( mFile ) { - QTextCodec *codec = QTextCodec::codecForName( mEncoding.toAscii() ); - mStream->setCodec( codec ); - } - mMaxRecordNumber = -1; - mHoldCurrentRecord = false; - if ( mWatcher ) delete mWatcher; - if ( mUseWatcher ) - { - mWatcher = new QFileSystemWatcher( this ); - mWatcher->addPath( mFileName ); - connect( mWatcher, SIGNAL( fileChanged( QString ) ), this, SLOT( updateFile() ) ); + mStream = new QTextStream( mFile ); + if ( ! mEncoding.isEmpty() ) + { + QTextCodec *codec = QTextCodec::codecForName( mEncoding.toAscii() ); + mStream->setCodec( codec ); + } + if ( mUseWatcher ) + { + mWatcher = new QFileSystemWatcher( this ); + mWatcher->addPath( mFileName ); + connect( mWatcher, SIGNAL( fileChanged( QString ) ), this, SLOT( updateFile() ) ); + } } } - return true; + return mFile != 0; } void QgsDelimitedTextFile::updateFile() @@ -501,6 +505,7 @@ int QgsDelimitedTextFile::fieldIndex( QString name ) bool QgsDelimitedTextFile::setNextRecordId( long nextRecordId ) { + if( ! mFile ) return false; mHoldCurrentRecord = nextRecordId == mRecordLineNumber; if ( mHoldCurrentRecord ) return true; return setNextLineNumber( nextRecordId ); @@ -524,7 +529,7 @@ QgsDelimitedTextFile::Status QgsDelimitedTextFile::nextRecord( QStringList &reco // Find the first non-blank line to read QString buffer; status = nextLine( buffer, true ); - if ( status != RecordOk ) return status; + if ( status != RecordOk ) return RecordEOF; mCurrentRecord.clear(); mRecordLineNumber = mLineNumber; diff --git a/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp b/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp index 781cdb5fdf0..e64a3e6a829 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp +++ b/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp @@ -60,6 +60,8 @@ QRegExp QgsDelimitedTextProvider::CrdDmsRegexp( "^\\s*(?:([-+nsew])\\s*)?(\\d{1, QgsDelimitedTextProvider::QgsDelimitedTextProvider( QString uri ) : QgsVectorDataProvider( uri ) + , mLayerValid( false ) + , mValid( false ) , mFile( 0 ) , mGeomRep( GeomNone ) , mFieldCount( 0 ) @@ -165,6 +167,29 @@ QgsDelimitedTextProvider::QgsDelimitedTextProvider( QString uri ) } } +QgsDelimitedTextProvider::~QgsDelimitedTextProvider() +{ + if ( mActiveIterator ) + mActiveIterator->close(); + + if ( mFile ) + { + delete mFile; + mFile = 0; + } + + if ( mSubsetExpression ) + { + delete mSubsetExpression; + mSubsetExpression = 0; + } + if ( mSpatialIndex ) + { + delete mSpatialIndex; + mSpatialIndex = 0; + } +} + QStringList QgsDelimitedTextProvider::readCsvtFieldTypes( QString filename, QString *message ) { // Look for a file with the same name as the data file, but an extra 't' or 'T' at the end @@ -241,7 +266,6 @@ QStringList QgsDelimitedTextProvider::readCsvtFieldTypes( QString filename, QStr } - void QgsDelimitedTextProvider::resetCachedSubset() { mCachedSubsetString = QString(); @@ -249,7 +273,6 @@ void QgsDelimitedTextProvider::resetCachedSubset() mCachedUseSpatialIndex = false; } - void QgsDelimitedTextProvider::resetIndexes() { resetCachedSubset(); @@ -276,14 +299,15 @@ bool QgsDelimitedTextProvider::createSpatialIndex() return true; } -// buildIndexes parameter of scanFile is to allow for potential rescan - if using -// subset string then rescan follows this to determine subset extents etc. -// Done this way as subset requires fields to be defined, which they are not -// until initial file scan is complete. -// -// Although at this point the subset expression does not apply (if one is defined) -// we still consider a subset index, as this also applies for implicit subsets -// due to filtering on geometry validity. +// Really want to merge scanFile and rescan into single code. Currently the reason +// this is not done is that scanFile is done initially to create field names and, rescan +// file includes building subset expression and assumes field names/types are already +// defined. Merging would not only make code a lot cleaner, but would also avoid +// double scan when loading a file with a subset expression. + +// buildIndexes parameter of scanFile is set to false when we know we will be +// immediately rescanning (when the file is loaded and then the subset expression is +// set) void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) { @@ -291,6 +315,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) // assume the layer is invalid until proven otherwise + mLayerValid = false; mValid = false; clearInvalidLines(); @@ -635,6 +660,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) mUseSpatialIndex = buildSpatialIndex; mValid = mGeometryType != QGis::UnknownGeometry; + mLayerValid = mValid; // If it is valid, then watch for changes to the file connect( mFile, SIGNAL( fileUpdated() ), this, SLOT( onFileUpdated() ) ); @@ -642,27 +668,97 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) } -QgsDelimitedTextProvider::~QgsDelimitedTextProvider() +// rescanFile. Called if something has changed file definition, such as +// selecting a subset, the file has been changed by another program, etc + +void QgsDelimitedTextProvider::rescanFile() { - if ( mActiveIterator ) - mActiveIterator->close(); + resetIndexes(); - if ( mFile ) + bool buildSpatialIndex = mSpatialIndex != 0; + bool buildSubsetIndex = mBuildSubsetIndex && ( mSubsetExpression || mGeomRep != GeomNone ); + + // In case file has been rewritten check that it is still valid + + mValid = mLayerValid && mFile->isValid(); + if ( ! mValid ) return; + + // Open the file and get number of rows, etc. We assume that the + // file has a header row and process accordingly. Caller should make + // sure that the delimited file is properly formed. + + QStringList messages; + + if ( mGeomRep == GeomAsWkt ) { - delete mFile; - mFile = 0; + mWktFieldIndex = mFile->fieldIndex( mWktFieldName ); + if ( mWktFieldIndex < 0 ) + { + messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "Wkt" ).arg( mWktFieldName ) ); + } + } + else if ( mGeomRep == GeomAsXy ) + { + mXFieldIndex = mFile->fieldIndex( mXFieldName ); + mYFieldIndex = mFile->fieldIndex( mYFieldName ); + if ( mXFieldIndex < 0 ) + { + messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "X" ).arg( mWktFieldName ) ); + } + if ( mYFieldIndex < 0 ) + { + messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "Y" ).arg( mWktFieldName ) ); + } + } + if ( messages.size() > 0 ) + { + reportErrors( messages, false ); + QgsDebugMsg( "Delimited text source invalid on rescan - missing geometry fields" ); + mValid = false; } - if ( mSubsetExpression ) + // Reset the field columns + + for ( int i = 0; i < attributeFields.size(); i++ ) { - delete mSubsetExpression; - mSubsetExpression = 0; + attributeColumns[i] = mFile->fieldIndex( attributeFields[i].name() ); } - if ( mSpatialIndex ) + + // Scan through the features in the file + + mSubsetIndex.clear(); + mUseSubsetIndex = false; + QgsFeatureIterator fi = getFeatures( QgsFeatureRequest() ); + mNumberFeatures = 0; + mExtent = QgsRectangle(); + QgsFeature f; + while ( fi.nextFeature( f ) ) { - delete mSpatialIndex; - mSpatialIndex = 0; + if ( mGeometryType != QGis::NoGeometry ) + { + if ( mNumberFeatures == 0 ) + { + mExtent = f.geometry()->boundingBox(); + } + else + { + QgsRectangle bbox( f.geometry()->boundingBox() ); + mExtent.combineExtentWith( &bbox ); + } + if ( buildSpatialIndex ) mSpatialIndex->insertFeature( f ); + } + if ( buildSubsetIndex ) mSubsetIndex.append(( quintptr ) f.id() ); + mNumberFeatures++; } + if ( buildSubsetIndex ) + { + long recordCount = mFile->recordCount(); + recordCount -= recordCount / SUBSET_ID_THRESHOLD_FACTOR; + mUseSubsetIndex = recordCount < mSubsetIndex.size(); + if ( ! mUseSubsetIndex ) mSubsetIndex.clear(); + } + + mUseSpatialIndex = buildSpatialIndex; } QgsGeometry *QgsDelimitedTextProvider::geomFromWkt( QString &sWkt ) @@ -688,7 +784,6 @@ QgsGeometry *QgsDelimitedTextProvider::geomFromWkt( QString &sWkt ) return geom; } - double QgsDelimitedTextProvider::dmsStringToDouble( const QString &sX, bool *xOk ) { static QString negative( "swSW-" ); @@ -725,7 +820,6 @@ double QgsDelimitedTextProvider::dmsStringToDouble( const QString &sX, bool *xOk return x; } - bool QgsDelimitedTextProvider::pointFromXY( QString &sX, QString &sY, QgsPoint &pt ) { if ( ! mDecimalPoint.isEmpty() ) @@ -756,8 +850,6 @@ bool QgsDelimitedTextProvider::pointFromXY( QString &sX, QString &sY, QgsPoint & return false; } - - QString QgsDelimitedTextProvider::storageType() const { return "Delimited text file"; @@ -770,6 +862,9 @@ void QgsDelimitedTextProvider::resetStream() QgsFeatureIterator QgsDelimitedTextProvider::getFeatures( const QgsFeatureRequest& request ) { + // If the file has become invalid, check that it is still invalid. + if( mLayerValid && ! mValid ) rescanFile(); + return QgsFeatureIterator( new QgsDelimitedTextFeatureIterator( this, request ) ); } @@ -845,8 +940,6 @@ void QgsDelimitedTextProvider::reportErrors( QStringList messages , bool showDia } } -// - bool QgsDelimitedTextProvider::setSubsetString( QString subset, bool updateFeatureCount ) { // If not changing string, then oll ok, nothing to do @@ -953,100 +1046,6 @@ void QgsDelimitedTextProvider::setUriParameter( QString parameter, QString value setDataSourceUri( QString::fromAscii( url.toEncoded() ) ); } -// rescanFile. Called if something has changed file definition, such as -// selecting a subset, the file has been changed by another program, etc - -void QgsDelimitedTextProvider::rescanFile() -{ - resetIndexes(); - - bool buildSpatialIndex = mSpatialIndex != 0; - bool buildSubsetIndex = mBuildSubsetIndex && ( mSubsetExpression || mGeomRep != GeomNone ); - - // In case file has been rewritten, check that required fields are still - // valid - - mValid = mFile->isValid(); - if ( ! mValid ) return; - - // Open the file and get number of rows, etc. We assume that the - // file has a header row and process accordingly. Caller should make - // sure that the delimited file is properly formed. - - QStringList messages; - - if ( mGeomRep == GeomAsWkt ) - { - mWktFieldIndex = mFile->fieldIndex( mWktFieldName ); - if ( mWktFieldIndex < 0 ) - { - messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "Wkt" ).arg( mWktFieldName ) ); - } - } - else if ( mGeomRep == GeomAsXy ) - { - mXFieldIndex = mFile->fieldIndex( mXFieldName ); - mYFieldIndex = mFile->fieldIndex( mYFieldName ); - if ( mXFieldIndex < 0 ) - { - messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "X" ).arg( mWktFieldName ) ); - } - if ( mYFieldIndex < 0 ) - { - messages.append( tr( "%0 field %1 is not defined in delimited text file" ).arg( "Y" ).arg( mWktFieldName ) ); - } - } - if ( messages.size() > 0 ) - { - reportErrors( messages, false ); - QgsDebugMsg( "Delimited text source invalid on rescan - missing geometry fields" ); - mValid = false; - } - - // Reset the field columns - - for ( int i = 0; i < attributeFields.size(); i++ ) - { - attributeColumns[i] = mFile->fieldIndex( attributeFields[i].name() ); - } - - // Scan through the features in the file - - mSubsetIndex.clear(); - mUseSubsetIndex = false; - QgsFeatureIterator fi = getFeatures( QgsFeatureRequest() ); - mNumberFeatures = 0; - mExtent = QgsRectangle(); - QgsFeature f; - while ( fi.nextFeature( f ) ) - { - if ( mGeometryType != QGis::NoGeometry ) - { - if ( mNumberFeatures == 0 ) - { - mExtent = f.geometry()->boundingBox(); - } - else - { - QgsRectangle bbox( f.geometry()->boundingBox() ); - mExtent.combineExtentWith( &bbox ); - } - if ( buildSpatialIndex ) mSpatialIndex->insertFeature( f ); - } - if ( buildSubsetIndex ) mSubsetIndex.append(( quintptr ) f.id() ); - mNumberFeatures++; - } - if ( buildSubsetIndex ) - { - long recordCount = mFile->recordCount(); - recordCount -= recordCount / SUBSET_ID_THRESHOLD_FACTOR; - mUseSubsetIndex = recordCount < mSubsetIndex.size(); - if ( ! mUseSubsetIndex ) mSubsetIndex.clear(); - } - - mUseSpatialIndex = buildSpatialIndex; -} - void QgsDelimitedTextProvider::onFileUpdated() { QStringList messages; @@ -1078,6 +1077,7 @@ bool QgsDelimitedTextProvider::nextFeature( QgsFeature& feature, QgsDelimitedTex QgsDelimitedTextFile::Status status = file->nextRecord( tokens ); if ( status == QgsDelimitedTextFile::RecordEOF ) break; if ( status != QgsDelimitedTextFile::RecordOk ) continue; + // We ignore empty records, such as added randomly by spreadsheets if ( recordIsEmpty( tokens ) ) continue; @@ -1153,7 +1153,6 @@ bool QgsDelimitedTextProvider::nextFeature( QgsFeature& feature, QgsDelimitedTex return false; } - QgsGeometry* QgsDelimitedTextProvider::loadGeometryWkt( const QStringList& tokens, QgsDelimitedTextFeatureIterator *iterator ) { QgsGeometry* geom = 0; @@ -1174,7 +1173,6 @@ QgsGeometry* QgsDelimitedTextProvider::loadGeometryWkt( const QStringList& token return geom; } - QgsGeometry* QgsDelimitedTextProvider::loadGeometryXY( const QStringList& tokens, QgsDelimitedTextFeatureIterator *iterator ) { QString sX = tokens[mXFieldIndex]; @@ -1189,7 +1187,6 @@ QgsGeometry* QgsDelimitedTextProvider::loadGeometryXY( const QStringList& tokens return 0; } - void QgsDelimitedTextProvider::fetchAttribute( QgsFeature& feature, int fieldIdx, const QStringList& tokens ) { if ( fieldIdx < 0 || fieldIdx >= attributeColumns.count() ) return; @@ -1272,10 +1269,9 @@ const QgsFields & QgsDelimitedTextProvider::fields() const bool QgsDelimitedTextProvider::isValid() { - return mValid; + return mLayerValid; } - int QgsDelimitedTextProvider::capabilities() const { return SelectAtId | CreateSpatialIndex; diff --git a/src/providers/delimitedtext/qgsdelimitedtextprovider.h b/src/providers/delimitedtext/qgsdelimitedtextprovider.h index 45a43f69931..b84826dfe4c 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextprovider.h +++ b/src/providers/delimitedtext/qgsdelimitedtextprovider.h @@ -239,8 +239,11 @@ class QgsDelimitedTextProvider : public QgsVectorDataProvider bool pointFromXY( QString &sX, QString &sY, QgsPoint &point ); double dmsStringToDouble( const QString &sX, bool *xOk ); - - QString mUri; + // mLayerValid defines whether the layer has been loaded as a valid layer + bool mLayerValid; + // mValid defines whether the layer is currently valid (may differ from + // mLayerValid if the file has been rewritten) + bool mValid; //! Text file QgsDelimitedTextFile *mFile; @@ -271,8 +274,6 @@ class QgsDelimitedTextProvider : public QgsVectorDataProvider //! Layer extent QgsRectangle mExtent; - bool mValid; - int mGeomType; long mNumberFeatures; diff --git a/tests/src/python/test_qgsdelimitedtextprovider.py b/tests/src/python/test_qgsdelimitedtextprovider.py index 11cb5bc30f8..1faf23327d4 100644 --- a/tests/src/python/test_qgsdelimitedtextprovider.py +++ b/tests/src/python/test_qgsdelimitedtextprovider.py @@ -28,6 +28,7 @@ import os.path; import re import tempfile import inspect +import time import test_qgsdelimitedtextprovider_wanted as want rebuildTests = 'REBUILD_DELIMITED_TEXT_TESTS' in os.environ; @@ -163,10 +164,11 @@ def delimitedTextData( testname, filename, requests, verbose, **params ): data = {} if layer.isValid(): for nr,r in enumerate(requests): - if verbose: - print "Processing request",nr+1,repr(r) + if verbose: print "Processing request",nr+1,repr(r) if callable(r): r( layer ) + if verbose: print "Request function executed" + if callable(r): continue rfields,rtypes, rdata = layerData(layer,r,nr*1000) if len(rfields) > len(fields): @@ -175,6 +177,8 @@ def delimitedTextData( testname, filename, requests, verbose, **params ): data.update(rdata) if not rdata: log.append("Request "+str(nr)+" did not return any data") + if verbose: + print "Request returned",len(rdata.keys()),"features" for msg in logger.messages(): filelogname = 'temp_file' if 'tmp' in filename.lower() else filename msg = re.sub(r'file\s+.*'+re.escape(filename),'file '+filelogname,msg) @@ -534,34 +538,42 @@ class TestQgsDelimitedTextProvider(TestCase): (filehandle,filename) = tempfile.mkstemp() with os.fdopen(filehandle,"w") as f: f.write("id,name\n1,rabbit\n2,pooh\n") - QCoreApplication.instance().processEvents() - def updatefile1( layer ): + def appendfile( layer ): with file(filename,'a') as f: f.write('3,tigger\n') + # print "Appended to file - sleeping" + time.sleep(1); QCoreApplication.instance().processEvents() - def updatefile2( layer ): + def rewritefile( layer ): with file(filename,'w') as f: f.write("name,size,id\ntoad,small,5\nmole,medium,6\nbadger,big,7\n") + # print "Rewritten file - sleeping" + time.sleep(1); QCoreApplication.instance().processEvents() def deletefile( layer ): os.remove(filename) + # print "Deleted file - sleeping" + time.sleep(1); + QCoreApplication.instance().processEvents() params={'geomType': 'none', 'type': 'csv', 'watchFile' : 'yes' } requests=[ {'fid': 3}, {}, {'fid': 7}, - updatefile1, + appendfile, {'fid': 3}, {'fid': 4}, {}, {'fid': 7}, - updatefile2, + rewritefile, {'fid': 2}, {}, {'fid': 7}, deletefile, {'fid': 2}, {}, + rewritefile, + {'fid': 2}, ] runTest(filename,requests,**params) diff --git a/tests/src/python/test_qgsdelimitedtextprovider_wanted.py b/tests/src/python/test_qgsdelimitedtextprovider_wanted.py index 2df8e409aef..609c28bc022 100644 --- a/tests/src/python/test_qgsdelimitedtextprovider_wanted.py +++ b/tests/src/python/test_qgsdelimitedtextprovider_wanted.py @@ -1346,32 +1346,22 @@ def test_029_file_watcher(): '#fid': 4L, '#geometry': 'None', }, - 13002L: { + 16002L: { 'id': u'5', 'description': u'toad', 'name': u'toad', '#fid': 2L, '#geometry': 'None', }, - 14003L: { - 'id': u'6', - 'description': u'mole', - 'name': u'mole', - '#fid': 3L, - '#geometry': 'None', - }, - 14004L: { - 'id': u'7', - 'description': u'badger', - 'name': u'badger', - '#fid': 4L, - '#geometry': 'None', - }, } wanted['log']=[ 'Request 2 did not return any data', 'Request 7 did not return any data', 'Request 11 did not return any data', + 'Request 13 did not return any data', + 'Request 14 did not return any data', + u'Errors in file temp_file', + u'The file has been updated by another application - reloading', u'Errors in file temp_file', u'The file has been updated by another application - reloading', u'Errors in file temp_file',