Skip to content

Commit

Permalink
Fix usage of database URLs in Importer Specification editor
Browse files Browse the repository at this point in the history
Importer was looking for wrong resource type when handling database URLs.
Also, we now call data source 'source', not 'filepath' in the Specification
editor.

Re spine-tools/Spine-Toolbox#2329
  • Loading branch information
soininen committed Nov 14, 2023
1 parent a365a86 commit 7e6ddbb
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 36 deletions.
18 changes: 11 additions & 7 deletions spine_items/importer/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,21 +214,25 @@ def _handle_files_double_clicked(self, index):
self.open_import_editor(index)

def open_import_editor(self, index):
"""Opens Import editor for the given index."""
filepath = None
"""Opens Import editor for the given index.
Args:
index (QModelIndex): resource list index
"""
source = None
if index.isValid():
resource = self._file_model.resource(index)
if resource.type_ == "database":
filepath = resource.url
if resource.type_ == "url":
source = resource.url
else:
if not resource.hasfilepath:
self._logger.msg_error.emit("File does not exist yet.")
else:
if not os.path.exists(resource.path):
self._logger.msg_error.emit(f"Cannot find file '{filepath}'.")
self._logger.msg_error.emit(f"Cannot find file '{source}'.")
else:
filepath = resource.path
self._toolbox.show_specification_form(self.item_type(), self.specification(), self, filepath=filepath)
source = resource.path
self._toolbox.show_specification_form(self.item_type(), self.specification(), self, source=source)

def select_connector_type(self, index):
"""Opens dialog to select connector type for the given index."""
Expand Down
4 changes: 2 additions & 2 deletions spine_items/importer/importer_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@ def make_specification_menu(parent, index):
@staticmethod
def make_specification_editor(toolbox, specification=None, item=None, **kwargs):
"""See base class."""
filepath = kwargs.get("filepath")
return ImportEditorWindow(toolbox, specification, item, filepath)
source = kwargs.get("source")
return ImportEditorWindow(toolbox, specification, item, source)
75 changes: 51 additions & 24 deletions spine_items/importer/widgets/import_editor_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,16 @@ def get_tables(self):
def get_data_iterator(self, table, options, max_rows=-1):
return iter([]), ()

def __init__(self, toolbox, specification, item=None, filepath=None):
def __init__(self, toolbox, specification, item=None, source=None):
"""
Args:
toolbox (QMainWindow): ToolboxUI class
specification (ImporterSpecification)
filepath (str, optional): Importee path
specification (ImporterSpecification, optional)
item (Importer, optional)
source (str, optional): Importee file path or URL
"""
super().__init__(toolbox, specification, item)
self._filepath = filepath if filepath else self._FILE_LESS
self._source = source if source else self._FILE_LESS
self._mappings_model = MappingsModel(self._undo_stack, self)
self._mappings_model.rowsInserted.connect(self._reselect_source_table)
self._ui.source_list.setModel(self._mappings_model)
Expand All @@ -99,8 +100,8 @@ def __init__(self, toolbox, specification, item=None, filepath=None):
self._import_mapping_options = ImportMappingOptions(self._mappings_model, self._ui, self._undo_stack)
self._import_sources = ImportSources(self._mappings_model, self._ui, self._undo_stack, self)
self._ui.comboBox_source_file.addItem(self._FILE_LESS)
if filepath:
self._ui.comboBox_source_file.addItem(filepath)
if source:
self._ui.comboBox_source_file.addItem(source)
self._ui.comboBox_source_file.setCurrentIndex(-1)
self._ui.comboBox_source_file.currentTextChanged.connect(self.start_ui)
self._ui.toolButton_browse_source_file.clicked.connect(self._show_open_file_dialog)
Expand All @@ -114,7 +115,7 @@ def __init__(self, toolbox, specification, item=None, filepath=None):
def showEvent(self, ev):
"""Select file path in the combobox, which calls the ``start_ui`` slot."""
super().showEvent(ev)
self._ui.comboBox_source_file.setCurrentText(self._filepath)
self._ui.comboBox_source_file.setCurrentText(self._source)

def is_file_less(self):
return self._ui.comboBox_source_file.currentText() == self._FILE_LESS
Expand All @@ -131,7 +132,7 @@ def _save(self, exiting=None):

@property
def _duplicate_kwargs(self):
return dict(filepath=self._filepath)
return dict(source=self._source)

def _make_ui(self):
from ..ui.import_editor_window import Ui_MainWindow # pylint: disable=import-outside-toplevel
Expand All @@ -156,13 +157,13 @@ def _populate_main_menu(self):
@Slot(bool)
def _show_open_file_dialog(self, _=False):
if self._connection_manager.connection.__name__ == "SqlAlchemyConnector":
filepath = self._get_source_url()
source = self._get_source_url()
else:
filepath = self._get_source_file_path()
if not filepath:
source = self._get_source_file_path()
if not source:
return
self._ui.comboBox_source_file.addItem(filepath)
self._ui.comboBox_source_file.setCurrentText(filepath)
self._ui.comboBox_source_file.addItem(source)
self._ui.comboBox_source_file.setCurrentText(source)

def _get_source_url(self):
selector = UrlSelectorDialog(self._toolbox.qsettings(), self._toolbox, parent=self)
Expand All @@ -189,7 +190,15 @@ def _switch_connector(self, _=False):
self._memoized_connector = None
self.start_ui(self._FILE_LESS)

def _get_connector_from_mapping(self, filepath):
def _get_connector_from_mapping(self, source):
"""Guesses connector for given source.
Args:
source (str): importee file path or URL
Returns:
type: connector class, or None if no suitable connector was found
"""
if not self.specification:
return None
mapping = self.specification.mapping
Expand All @@ -198,26 +207,28 @@ def _get_connector_from_mapping(self, filepath):
return None
connector = _CONNECTOR_NAME_TO_CLASS[source_type]
file_extensions = connector.FILE_EXTENSIONS.split(";;")
if filepath != self._FILE_LESS and not any(fnmatch.fnmatch(filepath, ext) for ext in file_extensions):
if source != self._FILE_LESS and not any(fnmatch.fnmatch(source, ext) for ext in file_extensions):
if isinstance(connector, SqlAlchemyConnector) and self._is_url(source):
return connector
return None
return connector

def start_ui(self, filepath):
def start_ui(self, source):
"""
Args:
filepath (str): Importee path
source (str): Importee path/URL
"""
connector = self._get_connector_from_mapping(filepath)
connector = self._get_connector_from_mapping(source)
if connector is None:
# Ask user
connector = self._get_connector(filepath)
connector = self._get_connector(source)
if not connector:
return
if connector.__name__ == "SqlAlchemyConnector":
self._ui.file_path_label.setText("URL")
else:
self._ui.file_path_label.setText("File path")
if filepath == self._FILE_LESS:
if source == self._FILE_LESS:
self._FileLessConnector.__name__ = connector.__name__
self._FileLessConnector.OPTIONS = connector.OPTIONS
connector = self._FileLessConnector
Expand All @@ -229,7 +240,7 @@ def start_ui(self, filepath):
if self._connection_manager:
self._connection_manager.close_connection()
self._connection_manager = ConnectionManager(connector, connector_settings, self)
self._connection_manager.source = filepath
self._connection_manager.source = source
self._connection_manager.connection_failed.connect(self.connection_failed.emit)
self._connection_manager.error.connect(self.show_error)
for header in (self._ui.source_data_table.horizontalHeader(), self._ui.source_data_table.verticalHeader()):
Expand All @@ -244,11 +255,11 @@ def _handle_connection_ready(self):
self._ui.export_mappings_action.setEnabled(True)
self._ui.import_mappings_action.setEnabled(True)

def _get_connector(self, filepath):
def _get_connector(self, source):
"""Shows a QDialog to select a connector for the given source file.
Args:
filepath (str): Path of the file acting as an importee
source (str): Path of the file acting as an importee
Returns:
Asynchronous data reader class for the given importee
Expand All @@ -265,8 +276,12 @@ def _get_connector(self, filepath):
row = None
for k, conn in enumerate(connector_list):
file_extensions = conn.FILE_EXTENSIONS.split(";;")
if any(fnmatch.fnmatch(filepath, ext) for ext in file_extensions):
if any(fnmatch.fnmatch(source, ext) for ext in file_extensions):
row = k
break
else:
if self._is_url(source):
row = connector_names.index(SqlAlchemyConnector.DISPLAY_NAME)
if row is not None:
connector_list_wg.setCurrentRow(row)
button_box = QDialogButtonBox(QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel)
Expand Down Expand Up @@ -341,6 +356,18 @@ def _reselect_source_table(self, parent, first, last):
index = self._mappings_model.index(last, 0)
self._ui.source_list.selectionModel().setCurrentIndex(index, QItemSelectionModel.ClearAndSelect)

@staticmethod
def _is_url(string):
"""Tests if given string looks like a URL.
Args:
string (str): string to test
Returns:
bool: True if string looks like a URL, False otherwise
"""
return "://" in string

def tear_down(self):
if not super().tear_down():
return False
Expand Down
6 changes: 3 additions & 3 deletions spine_items/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,13 @@ def set_checked(self, index, checked):
self.dataChanged.emit(index, index, [Qt.ItemDataRole.CheckStateRole])

def index_with_file_path(self):
"""Tries to find an item that has a valid file path.
"""Tries to find an item that has a valid file path or URL.
Returns:
QModelIndex: index to a item with usable file path or an invalid index if none found
QModelIndex: index to an item with usable file path/URL, or an invalid index if none found
"""
for row, item in enumerate(self._single_resources):
if item.checked and item.resource.hasfilepath:
if item.checked and item.resource.url:
return self.index(row, 0)
for pack_row, item in enumerate(self._pack_resources):
if item.checked:
Expand Down
57 changes: 57 additions & 0 deletions tests/importer/widgets/test_import_editor_window.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
######################################################################################################################
# Copyright (C) 2017-2022 Spine project consortium
# This file is part of Spine Items.
# Spine Items is free software: you can redistribute it and/or modify it under the terms of the GNU Lesser General
# Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option)
# any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
# without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General
# Public License for more details. You should have received a copy of the GNU Lesser General Public License along with
# this program. If not, see <http://www.gnu.org/licenses/>.
######################################################################################################################
"""Unit tests for the ``import_editor_window`` module."""
from tempfile import TemporaryDirectory
import unittest
from unittest import mock

from PySide6.QtWidgets import QApplication, QDialog

from spine_items.importer.widgets.import_editor_window import ImportEditorWindow
from spinedb_api.spine_io.importers.sqlalchemy_connector import SqlAlchemyConnector
from tests.mock_helpers import clean_up_toolbox, create_toolboxui_with_project


class TestIsUrl(unittest.TestCase):
def test_url_is_url(self):
self.assertTrue(ImportEditorWindow._is_url("sqlite:///C:\\data\\db.sqlite"))
self.assertTrue(ImportEditorWindow._is_url("postgresql+psycopg://spuser:[email protected]/db"))

def test_non_url_is_not_url(self):
self.assertFalse(ImportEditorWindow._is_url("C:\\path\\to\\file"))
self.assertFalse(ImportEditorWindow._is_url("/unix/style/path"))


class TestImportEditorWindow(unittest.TestCase):
@classmethod
def setUpClass(cls):
if not QApplication.instance():
QApplication()

def setUp(self):
self._temp_dir = TemporaryDirectory()
self._toolbox = create_toolboxui_with_project(self._temp_dir.name)

def tearDown(self):
clean_up_toolbox(self._toolbox)
self._temp_dir.cleanup()

def test_get_connector_selects_sql_alchemy_connector_when_source_is_url(self):
editor = ImportEditorWindow(self._toolbox, None)
with mock.patch("spine_items.importer.widgets.import_editor_window.QDialog.exec") as exec_dialog:
exec_dialog.return_value = QDialog.DialogCode.Accepted
connector = editor._get_connector("mysql://server.com/db")
exec_dialog.assert_called_once()
self.assertIs(connector, SqlAlchemyConnector)


if __name__ == '__main__':
unittest.main()

0 comments on commit 7e6ddbb

Please sign in to comment.