Skip to content

Commit

Permalink
Fix virtual layer definition parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
Hugo Mercier committed Jan 6, 2016
1 parent 41c9da9 commit 62cdb27
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 18 deletions.
14 changes: 7 additions & 7 deletions src/core/qgsvirtuallayerdefinition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinition::fromUrl( const QUrl& url )
QgsFields fields;

int layerIdx = 0;
QList<QPair<QString, QString> > items = url.queryItems();
QList<QPair<QByteArray, QByteArray> > items = url.encodedQueryItems();
for ( int i = 0; i < items.size(); i++ )
{
QString key = items.at( i ).first;
Expand Down Expand Up @@ -130,7 +130,7 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinition::fromUrl( const QUrl& url )
else if ( key == "query" )
{
// url encoded query
def.setQuery( value );
def.setQuery( QUrl::fromPercentEncoding( value.toUtf8() ) );
}
else if ( key == "field" )
{
Expand Down Expand Up @@ -171,11 +171,11 @@ QUrl QgsVirtualLayerDefinition::toUrl() const
if ( l.isReferenced() )
url.addQueryItem( "layer_ref", QString( "%1:%2" ).arg( l.reference(), l.name() ) );
else
url.addQueryItem( "layer", QString( "%1:%4:%2:%3" ) // the order is important, since the 4th argument may contain '%2' as well
.arg( l.provider(),
QString( QUrl::toPercentEncoding( l.name() ) ),
l.encoding(),
QString( QUrl::toPercentEncoding( l.source() ) ) ) );
url.addEncodedQueryItem( "layer", QString( "%1:%4:%2:%3" ) // the order is important, since the 4th argument may contain '%2' as well
.arg( l.provider(),
QString( QUrl::toPercentEncoding( l.name() ) ),
l.encoding(),
QString( QUrl::toPercentEncoding( l.source() ) ) ).toUtf8() );
}

if ( !query().isEmpty() )
Expand Down
6 changes: 5 additions & 1 deletion src/providers/virtual/qgsvirtuallayersqlitemodule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,14 @@ struct VTable
, mValid( true )
{
mProvider = static_cast<QgsVectorDataProvider*>( QgsProviderRegistry::instance()->provider( provider, source ) );
if ( !mProvider || !mProvider->isValid() )
if ( !mProvider )
{
throw std::runtime_error( "Invalid provider" );
}
else if ( mProvider && !mProvider->isValid() )
{
throw std::runtime_error(( "Provider error:" + mProvider->error().message() ).toUtf8().constData() );
}
if ( mProvider->capabilities() & QgsVectorDataProvider::SelectEncoding )
{
mProvider->setEncoding( mEncoding );
Expand Down
22 changes: 13 additions & 9 deletions tests/src/python/test_provider_virtual.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
QgsRectangle,
QgsErrorMessage,
QgsProviderRegistry,
QgsVirtualLayerDefinition
QgsVirtualLayerDefinition,
QgsWKBTypes
)

from utilities import (unitTestDataPath,
Expand Down Expand Up @@ -84,7 +85,7 @@ def tearDown(self):
pass

def test_CsvNoGeometry(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand All @@ -95,7 +96,7 @@ def test_CsvNoGeometry(self):

def test_source_escaping(self):
# the source contains ':'
source = "file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no"
source = "file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no"
d = QgsVirtualLayerDefinition()
d.addSource("t", source, "delimitedtext")
l = QgsVectorLayer(d.toString(), "vtab", "virtual", False)
Expand Down Expand Up @@ -134,7 +135,7 @@ def create_test_db(dbfile):
self.assertEqual(l.isValid(), True)

def test_DynamicGeometry(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/testextpt.txt") + "?type=csv&delimiter=%7C&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/testextpt.txt").replace("\\", "/") + "?type=csv&delimiter=%7C&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand Down Expand Up @@ -327,7 +328,10 @@ def test_recursiveLayer(self):

def test_no_geometry(self):
source = QUrl.toPercentEncoding(os.path.join(self.testDataDir, "france_parts.shp"))
l2 = QgsVectorLayer("?layer=ogr:%s:vtab&nogeometry" % source, "vtab2", "virtual", False)
df = QgsVirtualLayerDefinition()
df.addSource("vtab", os.path.join(self.testDataDir, "france_parts.shp"), "ogr")
df.setGeometryWkbType(QgsWKBTypes.NoGeometry)
l2 = QgsVectorLayer(df.toString(), "vtab2", "virtual", False)
self.assertEqual(l2.isValid(), True)
self.assertEqual(l2.dataProvider().geometryType(), 100) # NoGeometry

Expand Down Expand Up @@ -386,7 +390,7 @@ def test_reopen4(self):
self.assertEqual(suma, 3064.0)

def test_refLayer(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand All @@ -399,7 +403,7 @@ def test_refLayer(self):
print sum([f.id() for f in l2.getFeatures()])

def test_refLayers(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand All @@ -414,7 +418,7 @@ def test_refLayers(self):
QgsMapLayerRegistry.instance().removeMapLayer(l2.id())

def test_refLayers2(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand All @@ -425,7 +429,7 @@ def test_refLayers2(self):
self.assertEqual("Cannot store referenced layers" in l2.dataProvider().error().message(), True)

def test_sql(self):
l1 = QgsVectorLayer(os.path.join(self.testDataDir, "delimitedtext/test.csv") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
l1 = QgsVectorLayer("file:///" + os.path.join(self.testDataDir, "delimitedtext/test.csv").replace("\\", "/") + "?type=csv&geomType=none&subsetIndex=no&watchFile=no", "test", "delimitedtext", False)
self.assertEqual(l1.isValid(), True)
QgsMapLayerRegistry.instance().addMapLayer(l1)

Expand Down
15 changes: 14 additions & 1 deletion tests/src/python/test_qgsvirtuallayerdefinition.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from utilities import (TestCase, unittest)

from PyQt4.QtCore import QVariant
from PyQt4.QtCore import QVariant, QUrl


class TestQgsVirtualLayerDefinition(TestCase):
Expand All @@ -34,32 +34,45 @@ def test1(self):
d.setFilePath("/file")
self.assertEqual(d.toString(), "/file")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).filePath(), "/file")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).filePath(), "/file")
d.setFilePath("C:\\file")
self.assertEqual(d.toString(), "C:%5Cfile")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).filePath(), "C:\\file")
d.setQuery("SELECT * FROM mytable")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).query(), "SELECT * FROM mytable")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).query(), "SELECT * FROM mytable")

q = u"SELECT * FROM tableéé /*:int*/"
d.setQuery(q)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).query(), q)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).query(), q)

s1 = u"file://foo&bar=okié"
d.addSource("name", s1, "provider", "utf8")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).sourceLayers()[0].source(), s1)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).sourceLayers()[0].source(), s1)

n1 = u"éé ok"
d.addSource(n1, s1, "provider")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).sourceLayers()[1].name(), n1)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).sourceLayers()[1].name(), n1)

d.addSource("ref1", "id0001")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).sourceLayers()[2].reference(), "id0001")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).sourceLayers()[2].reference(), "id0001")

s = "dbname='C:\\tt' table=\"test\" (geometry) sql="
d.addSource("nn", s, "spatialite")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).sourceLayers()[3].source(), s)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).sourceLayers()[3].source(), s)

d.setGeometryField("geom")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).geometryField(), "geom")
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).geometryField(), "geom")

d.setGeometryWkbType(QgsWKBTypes.Point)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(d.toUrl()).geometryWkbType(), QgsWKBTypes.Point)
self.assertEqual(QgsVirtualLayerDefinition.fromUrl(QUrl.fromEncoded(d.toString())).geometryWkbType(), QgsWKBTypes.Point)

f = QgsFields()
f.append(QgsField("a", QVariant.Int))
Expand Down

1 comment on commit 62cdb27

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugo I still see the same test failures under windows with this commit.

Here's the results of my other tests:

  • add a Postgres layer with 50 features
  • create a virtual layer with a ' where name like 'M%' ' filter
  • under windows, this hangs QGIS for about 30 seconds when trying to open the attribute table.
  • under linux, the attribute table opens quickly but all values are set to 'ERROR'

Repeating the tests with a layer with 1 million features (with an indexed attribute filter which should result in about 10 matches)

  • linux hangs for about 5 minutes when trying to add the layer and on any operations (eg redraw)
  • under windows I can add the layer, but trying to open attribute table results in the app entering a semi-responding, broken state for 10 minutes before the table opens
    (I suspect loading this layer is just too expensive?)

Please sign in to comment.