Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with database URL checks #230

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 23 additions & 26 deletions spine_items/data_store/data_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,14 @@ def __init__(self, name, description, x, y, toolbox, project, url):
self._purge_settings = None
self._purge_dialog = None
self._database_validator = DatabaseConnectionValidator(self)
db_map = self.get_db_map_for_ds()
# Notify db manager about the Data Stores in the project, so it can notify about the dirtiness of them
self._toolbox.db_mngr.add_data_store_db_map(db_map, self)
self._toolbox.db_mngr.database_clean_changed.connect(self._set_database_clean)

def get_db_map_for_ds(self):
def get_db_map(self):
"""Returns the db map for the Data Store"""
sa_url = self.sql_alchemy_url()
if sa_url is not None:
return self._toolbox.db_mngr.get_db_map(sa_url, self._logger, codename=self.name)
return None
if sa_url is None:
return None
return self._toolbox.db_mngr.get_db_map(sa_url, self._logger, codename=self.name)

@staticmethod
def item_type():
Expand Down Expand Up @@ -372,13 +370,20 @@ def _check_notifications(self):
self._database_validator.validate_url(
self._url["dialect"], sa_url, self._set_invalid_url_notification, self._accept_url
)
db_map = self.get_db_map_for_ds()
if db_map:
clean = not self._toolbox.db_mngr.is_dirty(db_map)
self.notify_about_dirtiness(clean)

@Slot(bool)
def notify_about_dirtiness(self, clean):
@Slot(object, bool)
def _set_database_clean(self, db_map, clean):
"""Updates database cleanliness status.

Args:
db_map (DatabaseMapping): database mapping
clean (bool): True if database is clean, False otherwise
"""
if db_map is not self.get_db_map():
return
self._notify_about_dirtiness(clean)

def _notify_about_dirtiness(self, clean):
"""
Handles the notification for the dirtiness of the Data Store

Expand Down Expand Up @@ -422,7 +427,10 @@ def _accept_url(self, url):
self._resources_to_predecessors_changed()
self._resources_to_successors_changed()
self._update_actions_enabled()
self._toolbox.db_mngr.update_data_store_db_maps()
db_map = self.get_db_map()
if db_map:
clean = not self._toolbox.db_mngr.is_dirty(db_map)
self._notify_about_dirtiness(clean)

def is_url_validated(self):
"""Tests whether the URL has been validated.
Expand Down Expand Up @@ -485,7 +493,6 @@ def rename(self, new_name, rename_data_dir_message):
"""See base class."""
old_data_dir = os.path.abspath(self.data_dir) # Old data_dir before rename
old_name = self.name
self.rename_data_store_in_db_mngr(old_name) # Notify db manager about the rename
if not super().rename(new_name, rename_data_dir_message):
return False
# If dialect is sqlite and db line edit refers to a file in the old data_dir, db line edit needs updating
Expand Down Expand Up @@ -534,18 +541,8 @@ def resources_for_direct_predecessors(self):
"""See base class."""
return self.resources_for_direct_successors()

def rename_data_store_in_db_mngr(self, old_name):
"""Renames the Data Store in the used db manager"""
db_map = self.get_db_map_for_ds()
self._toolbox.db_mngr.update_data_store_db_maps()
index = next((i for i, store in enumerate(self._toolbox.db_mngr.data_stores[db_map]) if store.name == old_name))
if index is not None:
self._toolbox.db_mngr.data_stores[db_map][index] = self

def tear_down(self):
"""See base class"""
self._toolbox.db_mngr.database_clean_changed.disconnect(self._set_database_clean)
self._database_validator.wait_for_finish()
db_map = self.get_db_map_for_ds()
if db_map is not None:
self._toolbox.db_mngr.remove_data_store_db_map(db_map, self)
super().tear_down()
5 changes: 1 addition & 4 deletions tests/data_store/test_dataStore.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ def test_rename(self):
self.assertTrue(os.path.exists(url["database"]))

def test_dirty_db_notification(self):
"""Tests renaming a Data Store with an existing sqlite db in it's data_dir."""
temp_path = self.create_temp_db()
url = {"dialect": "sqlite", "database": temp_path}
self.ds._url = self.ds.parse_url(url)
Expand All @@ -132,15 +131,13 @@ def test_dirty_db_notification(self):
self.ds._check_notifications()
self.assertEqual([], self.ds.get_icon().exclamation_icon._notifications)
# Check that there is a warning about uncommitted changes
db_map = self.ds.get_db_map_for_ds()
db_map = self.ds.get_db_map()
self._toolbox.db_mngr.add_entity_classes({db_map: [{"name": "my_object_class"}]})
self.ds._check_notifications()
self.assertEqual(
[f"{self.ds.name} has uncommitted changes"], self.ds.get_icon().exclamation_icon._notifications
)
# Check that the warning disappears after committing the changes
self._toolbox.db_mngr.commit_session("Added entity classes", db_map)
self.ds._check_notifications()
self.assertEqual([], self.ds.get_icon().exclamation_icon._notifications)


Expand Down
Loading