sharpetronics/QGIS
1
0
Fork 0
mirror of https://github.com/qgis/QGIS.git synced 2025-02-25 00:58:06 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix to potential unterminated loop if a delimited text file is deleted

Browse Source 
Another try at fixing test cases with file watcher, don't work on some
platforms.
This commit is contained in:
Chris Crook 2013-06-02 08:49:04 +12:00
parent a39b78bb3e
commit e87623d39e
5 changed files with 173 additions and 169 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
src/providers/delimitedtext/qgsdelimitedtextfile.cpp
View File

@ -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;

250
src/providers/delimitedtext/qgsdelimitedtextprovider.cpp
View File

@ -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;

9
src/providers/delimitedtext/qgsdelimitedtextprovider.h
View File

@ -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;

26
tests/src/python/test_qgsdelimitedtextprovider.py
View File

@ -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)

20
tests/src/python/test_qgsdelimitedtextprovider_wanted.py
View File

@ -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',