Skip to content

Commit 6c53641

Browse files
strkstrk
committedOct 15, 2016
Fix extraction of ogr LayerName from multi-layer dataset URIs
Adds supports for "layerid" when present. Drop special handling for "table=" portions found in URI, making the code more generic. Includes testcase. Fixes #15698 - import geodatabase to postgis via processing
·
ltr-3_40final-3_0_0
1 parent 60b4b4d commit 6c53641

File tree

2 files changed

+78
-14
lines changed

2 files changed

+78
-14
lines changed
 

‎python/plugins/processing/tests/ToolsTest.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,67 @@
3131
from qgis.core import (QgsVectorLayer, QgsFeatureRequest)
3232
from processing.core.ProcessingConfig import ProcessingConfig
3333

34+
import os.path
35+
import errno
36+
import shutil
37+
38+
dataFolder = os.path.join(os.path.dirname(__file__), '../../../../tests/testdata/')
39+
tmpBaseFolder = os.path.join(os.sep, 'tmp', 'qgis_test', str(os.getpid()))
40+
41+
42+
def mkDirP(path):
43+
try:
44+
os.makedirs(path)
45+
except OSError as exc:
46+
if exc.errno == errno.EEXIST and os.path.isdir(path):
47+
pass
48+
else:
49+
raise
50+
3451
start_app()
3552

3653

3754
class VectorTest(unittest.TestCase):
3855

56+
@classmethod
57+
def setUpClass(cls):
58+
mkDirP(tmpBaseFolder)
59+
60+
@classmethod
61+
def tearDownClass(cls):
62+
shutil.rmtree(tmpBaseFolder)
63+
pass
64+
65+
# See http://hub.qgis.org/issues/15698
66+
def test_ogrLayerName(self):
67+
tmpdir = os.path.join(tmpBaseFolder, 'ogrLayerName')
68+
os.mkdir(tmpdir)
69+
70+
def linkTestfile(f, t):
71+
os.link(os.path.join(dataFolder, f), os.path.join(tmpdir, t))
72+
73+
linkTestfile('geom_data.csv', 'a.csv')
74+
name = vector.ogrLayerName(tmpdir)
75+
self.assertEqual(name, 'a')
76+
77+
linkTestfile('wkt_data.csv', 'b.csv')
78+
name = vector.ogrLayerName(tmpdir + '|layerid=0')
79+
self.assertEqual(name, 'a')
80+
name = vector.ogrLayerName(tmpdir + '|layerid=1')
81+
self.assertEqual(name, 'b')
82+
83+
name = vector.ogrLayerName(tmpdir + '|layerid=2')
84+
self.assertEqual(name, 'invalid-layerid')
85+
86+
name = vector.ogrLayerName(tmpdir + '|layername=f')
87+
self.assertEqual(name, 'f') # layername takes precedence
88+
89+
name = vector.ogrLayerName(tmpdir + '|layerid=0|layername=f2')
90+
self.assertEqual(name, 'f2') # layername takes precedence
91+
92+
name = vector.ogrLayerName(tmpdir + '|layername=f2|layerid=0')
93+
self.assertEqual(name, 'f2') # layername takes precedence
94+
3995
def testFeatures(self):
4096
ProcessingConfig.initialize()
4197

‎python/plugins/processing/tools/vector.py

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141

4242
import psycopg2
4343

44+
from osgeo import ogr
45+
4446
from qgis.PyQt.QtCore import QVariant, QSettings
4547
from qgis.core import (Qgis, QgsFields, QgsField, QgsGeometry, QgsRectangle, QgsWkbTypes,
4648
QgsSpatialIndex, QgsMapLayerRegistry, QgsMapLayer, QgsVectorLayer,
@@ -531,20 +533,26 @@ def ogrConnectionString(uri):
531533

532534

533535
def ogrLayerName(uri):
534-
if 'host' in uri:
535-
regex = re.compile('(table=")(.+?)(\.)(.+?)"')
536-
r = regex.search(uri)
537-
return '"' + r.groups()[1] + '.' + r.groups()[3] + '"'
538-
elif 'dbname' in uri:
539-
regex = re.compile('(table=")(.+?)"')
540-
r = regex.search(uri)
541-
return r.groups()[1]
542-
elif 'layername' in uri:
543-
regex = re.compile('(layername=)(.*)')
544-
r = regex.search(uri)
545-
return r.groups()[1]
546-
else:
547-
return os.path.basename(os.path.splitext(uri)[0])
536+
fields = uri.split('|')
537+
ogruri = fields[0]
538+
fields = fields[1:]
539+
layerid = 0
540+
for f in fields:
541+
if f.startswith('layername='):
542+
# Name encoded in uri, nothing more needed
543+
return f.split('=')[1]
544+
if f.startswith('layerid='):
545+
layerid = int(f.split('=')[1])
546+
# Last layerid= takes precedence, to allow of layername to
547+
# take precedence
548+
ds = ogr.Open(ogruri)

Comment on line R548

gacarrillor commented on Oct 15, 2016

@gacarrillor
gacarrillor
on Oct 15, 2016
Member

@strk, I think this will not be able to open a URI like 'dbname=\'/tmp/test.sqlite\' table="test" (geometry) sql=', returning "invalid-uri", although "test" would be expected by anyone calling ogrLayerName.

ogrLayerName gets URIs from QGIS layers. Please fix this so I can send a PR that will hopefully enter into QGIS v2.18.

strk replied on Oct 17, 2016

@strk
strk
on Oct 17, 2016
ContributorAuthor

strk replied on Oct 17, 2016

@strk
strk
on Oct 17, 2016
ContributorAuthor

@gacarrillor can you show example code ending up passing that kind of URI to the OGR provider ?
I ask because if I open an sqlite file like the one found under the testsuite and double-click on the layer I get a "Source layer" in the properties which is like this: /usr/src/qgis/qgis-2.18/tests/testdata/qgis_local_server/test-project/features_v3.sqlite|layerid=1

strk replied on Oct 17, 2016

@strk
strk
on Oct 17, 2016
ContributorAuthor

In other words, can you show any of the ogrLayerName callers possibly passing the URI you showed as an example ?

gacarrillor replied on Oct 17, 2016

@gacarrillor
gacarrillor
on Oct 17, 2016
Member

Yes, imagine that you load a Spatialite table to QGIS (via setting a connection and selecting a table) and then you run the "Convert format" tool (could be any OGR algorithm) using such layer as parameter. This will call ogrLayerName with the aforementioned URI at https://github.com/qgis/QGIS/blob/master_2/python/plugins/processing/algs/gdal/ogr2ogr.py#L141

I'm working on a related issue here. I'll take your commit as a basis but resort to the old if clauses right before returning "invalid-uri". This is how it looks now. I'll push the commit in some hours.

strk replied on Oct 17, 2016

@strk
strk
on Oct 17, 2016
ContributorAuthor

gacarrillor replied on Oct 17, 2016

@gacarrillor
gacarrillor
on Oct 17, 2016
Member

I'll have a look at it!

Code has comments. Press enter to view.
549+
if not ds:
550+
return "invalid-uri"
551+
ly = ds.GetLayer(layerid)
552+
if not ly:
553+
return "invalid-layerid"
554+
name = ly.GetName()
555+
return name
548556

549557

550558
class VectorWriter(object):

3 commit comments

Comments
 (3)

alexbruy commented on Oct 31, 2016

@alexbruy
alexbruy
on Oct 31, 2016
Contributor

This commit breaks all OGR geoprocessing tools. To reproduce:

  1. open polys.gml from Processing test dataset
  2. try to execute "Buffer vectors" from GDAL/OGR -> [OGR] Geoprocessing -> Buffer vectors
  3. algorithm execution will fail with error
Warning 1: layer names ignored in combination with -sql. 
ERROR 1: In ExecuteSQL(): sqlite3_prepare(SELECT ST_Buffer(geometry, 1000), * FROM 'polys2'): 
no such table: polys2

Also related tests in ToolsTest.py fail if /tmp located on partition different from partition where QGIS sources and build directory located.

strk commented on Oct 31, 2016

@strk
strk
on Oct 31, 2016
ContributorAuthor

alexbruy commented on Oct 31, 2016

@alexbruy
alexbruy
on Oct 31, 2016
Contributor

And travis cought none of them ? That's scary !

Maybe this is because we have no tests for this algs?

Please sign in to comment.