Skip to content

Commit

Permalink
Fix extraction of ogr LayerName from multi-layer dataset URIs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
strk committed Oct 15, 2016
1 parent 60b4b4d commit 6c53641
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 14 deletions.
56 changes: 56 additions & 0 deletions python/plugins/processing/tests/ToolsTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,67 @@
from qgis.core import (QgsVectorLayer, QgsFeatureRequest)
from processing.core.ProcessingConfig import ProcessingConfig

import os.path
import errno
import shutil

dataFolder = os.path.join(os.path.dirname(__file__), '../../../../tests/testdata/')
tmpBaseFolder = os.path.join(os.sep, 'tmp', 'qgis_test', str(os.getpid()))


def mkDirP(path):
try:
os.makedirs(path)
except OSError as exc:
if exc.errno == errno.EEXIST and os.path.isdir(path):
pass
else:
raise

start_app()


class VectorTest(unittest.TestCase):

@classmethod
def setUpClass(cls):
mkDirP(tmpBaseFolder)

@classmethod
def tearDownClass(cls):
shutil.rmtree(tmpBaseFolder)
pass

# See http://hub.qgis.org/issues/15698
def test_ogrLayerName(self):
tmpdir = os.path.join(tmpBaseFolder, 'ogrLayerName')
os.mkdir(tmpdir)

def linkTestfile(f, t):
os.link(os.path.join(dataFolder, f), os.path.join(tmpdir, t))

linkTestfile('geom_data.csv', 'a.csv')
name = vector.ogrLayerName(tmpdir)
self.assertEqual(name, 'a')

linkTestfile('wkt_data.csv', 'b.csv')
name = vector.ogrLayerName(tmpdir + '|layerid=0')
self.assertEqual(name, 'a')
name = vector.ogrLayerName(tmpdir + '|layerid=1')
self.assertEqual(name, 'b')

name = vector.ogrLayerName(tmpdir + '|layerid=2')
self.assertEqual(name, 'invalid-layerid')

name = vector.ogrLayerName(tmpdir + '|layername=f')
self.assertEqual(name, 'f') # layername takes precedence

name = vector.ogrLayerName(tmpdir + '|layerid=0|layername=f2')
self.assertEqual(name, 'f2') # layername takes precedence

name = vector.ogrLayerName(tmpdir + '|layername=f2|layerid=0')
self.assertEqual(name, 'f2') # layername takes precedence

def testFeatures(self):
ProcessingConfig.initialize()

Expand Down
36 changes: 22 additions & 14 deletions python/plugins/processing/tools/vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

import psycopg2

from osgeo import ogr

from qgis.PyQt.QtCore import QVariant, QSettings
from qgis.core import (Qgis, QgsFields, QgsField, QgsGeometry, QgsRectangle, QgsWkbTypes,
QgsSpatialIndex, QgsMapLayerRegistry, QgsMapLayer, QgsVectorLayer,
Expand Down Expand Up @@ -531,20 +533,26 @@ def ogrConnectionString(uri):


def ogrLayerName(uri):
if 'host' in uri:
regex = re.compile('(table=")(.+?)(\.)(.+?)"')
r = regex.search(uri)
return '"' + r.groups()[1] + '.' + r.groups()[3] + '"'
elif 'dbname' in uri:
regex = re.compile('(table=")(.+?)"')
r = regex.search(uri)
return r.groups()[1]
elif 'layername' in uri:
regex = re.compile('(layername=)(.*)')
r = regex.search(uri)
return r.groups()[1]
else:
return os.path.basename(os.path.splitext(uri)[0])
fields = uri.split('|')
ogruri = fields[0]
fields = fields[1:]
layerid = 0
for f in fields:
if f.startswith('layername='):
# Name encoded in uri, nothing more needed
return f.split('=')[1]
if f.startswith('layerid='):
layerid = int(f.split('=')[1])
# Last layerid= takes precedence, to allow of layername to
# take precedence
ds = ogr.Open(ogruri)

This comment has been minimized.

Copy link
@gacarrillor

gacarrillor 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.

This comment has been minimized.

Copy link
@strk

strk via email Oct 17, 2016

Author Contributor

This comment has been minimized.

Copy link
@strk

strk Oct 17, 2016

Author Contributor

@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

This comment has been minimized.

Copy link
@strk

strk Oct 17, 2016

Author Contributor

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

This comment has been minimized.

Copy link
@gacarrillor

gacarrillor 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.

This comment has been minimized.

Copy link
@strk

strk via email Oct 17, 2016

Author Contributor

This comment has been minimized.

Copy link
@gacarrillor

gacarrillor Oct 17, 2016

Member

I'll have a look at it!

if not ds:
return "invalid-uri"
ly = ds.GetLayer(layerid)
if not ly:
return "invalid-layerid"
name = ly.GetName()
return name


class VectorWriter(object):
Expand Down

3 comments on commit 6c53641

@alexbruy
Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

@strk strk commented on 6c53641 Oct 31, 2016 via email

Choose a reason for hiding this comment

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

@alexbruy
Copy link
Contributor

@alexbruy alexbruy commented on 6c53641 Oct 31, 2016

Choose a reason for hiding this comment

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

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.