dbaccess/Module_dbaccess.mk | 1 dbaccess/UITest_query.mk | 20 ++ dbaccess/qa/uitest/data/tdf99619.odb |binary dbaccess/qa/uitest/query/tdf99619_create_join_undo_redo.py | 96 +++++++++++++ dbaccess/source/ui/querydesign/QueryTableView.cxx | 16 +- 5 files changed, 127 insertions(+), 6 deletions(-)
New commits: commit f016c1537d36f7da42fb26d0322e0d4fdaa1f551 Author: Neil Roberts <[email protected]> AuthorDate: Sun Nov 9 12:00:57 2025 +0100 Commit: Noel Grandin <[email protected]> CommitDate: Tue Nov 11 06:46:47 2025 +0100 tdf#99619 Add a UI test The UI test opens a database with two tables, creates a join between the tables and then undos and redos the join. Change-Id: I4ad03435ce9c6484f54aa563b46d868f21a47209 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193762 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins diff --git a/dbaccess/Module_dbaccess.mk b/dbaccess/Module_dbaccess.mk index 27da2e72c70e..035c0445c62c 100644 --- a/dbaccess/Module_dbaccess.mk +++ b/dbaccess/Module_dbaccess.mk @@ -92,6 +92,7 @@ endif $(eval $(call gb_Module_add_uicheck_targets,dbaccess,\ UITest_edit_field \ + UITest_query \ )) endif diff --git a/dbaccess/UITest_query.mk b/dbaccess/UITest_query.mk new file mode 100644 index 000000000000..49a0ece17ce1 --- /dev/null +++ b/dbaccess/UITest_query.mk @@ -0,0 +1,20 @@ +# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*- +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# + +$(eval $(call gb_UITest_UITest,query)) + +$(eval $(call gb_UITest_add_modules,query,$(SRCDIR)/dbaccess/qa/uitest,\ + query/ \ +)) + +$(eval $(call gb_UITest_set_defs,query, \ + TDOC="$(SRCDIR)/dbaccess/qa/uitest/data" \ +)) + +# vim: set noet sw=4 ts=4: diff --git a/dbaccess/qa/uitest/data/tdf99619.odb b/dbaccess/qa/uitest/data/tdf99619.odb new file mode 100644 index 000000000000..e76b02b703a6 Binary files /dev/null and b/dbaccess/qa/uitest/data/tdf99619.odb differ diff --git a/dbaccess/qa/uitest/query/tdf99619_create_join_undo_redo.py b/dbaccess/qa/uitest/query/tdf99619_create_join_undo_redo.py new file mode 100644 index 000000000000..bdbccf8bb3fe --- /dev/null +++ b/dbaccess/qa/uitest/query/tdf99619_create_join_undo_redo.py @@ -0,0 +1,96 @@ +# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*- +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +from uitest.framework import UITestCase +from uitest.uihelper.common import get_url_for_data_file, get_state_as_dict, select_by_text +from uitest.test import DEFAULT_SLEEP + +from com.sun.star.util import URL + +import sys +import time + +#Bug 99619 - query design segfault on redoing an undone table join creation +class tdf99619(UITestCase): + def execute_for_provider(self, xProvider, command): + url = URL() + url.Complete = command + xUrlTransformer = self.xContext.ServiceManager.createInstanceWithContext( + "com.sun.star.util.URLTransformer", self.xContext) + _, url = xUrlTransformer.parseStrict(url) + + xDispatch = xProvider.queryDispatch(url, "", 0) + xDispatch.dispatch(url, []) + + def test_tdf99619(self): + # The sample file is an HSQLDB database with the two tables “person” and “object”. They both + # have a text field and a primary key. The object table additionally has an person_id field + # which is meant to reference an owner in the person table. There is also one query called + # “object_owners” which contains the two tables but no join. + with self.ui_test.load_file(get_url_for_data_file("tdf99619.odb")) as document: + xDbWindow = self.xUITest.getTopFocusWindow() + + self.xUITest.executeCommand(".uno:DBViewQueries") + + # We just want to select the single query in the list of queries but it seems difficult + # to get access to the treeview window for it because there are multiple treeviews in a + # hierarchy of windows with no name. However AppController handles SelectAll and that + # does the trick + self.xUITest.executeCommand(".uno:SelectAll") + + xDbFrame = self.ui_test.get_desktop().getCurrentFrame() + + self.xUITest.executeCommand(".uno:DBQueryEdit") + + while True: + xQueryFrame = self.ui_test.get_desktop().getCurrentFrame() + + if xQueryFrame != xDbFrame: + break + time.sleep(DEFAULT_SLEEP) + + xQueryController = xQueryFrame.getController() + + # Add a relation via the dialog + with self.ui_test.execute_blocking_action( + self.execute_for_provider, + args=(xQueryController, ".uno:DBAddRelation")) as xDialog: + + # Choose the two tables + select_by_text(xDialog.getChild("table1"), "object") + select_by_text(xDialog.getChild("table2"), "person") + + # Set the join type + select_by_text(xDialog.getChild("type"), "Inner join") + + # Use a natural join because it’s too difficult to manipulate the grid to select + # fields + xDialog.getChild("natural").executeAction("CLICK", tuple()) + + # Undo the join + self.execute_for_provider(xQueryFrame, ".uno:Undo") + # Redo the join. This is where it crashes without any fixes to the bug + self.execute_for_provider(xQueryFrame, ".uno:Redo") + + # Save the query. This only saves the query in memory and doesn’t change the database + # file on disk + self.execute_for_provider(xQueryFrame, ".uno:Save") + + # Switch to SQL mode + self.execute_for_provider(xQueryController, ".uno:DBChangeDesignMode") + + # Get the SQL source for the query + xSql = self.xUITest.getTopFocusWindow().getChild("sql") + query = get_state_as_dict(xSql)["Text"] + + # Make sure that the join is in the query + if "NATURAL INNER JOIN" not in query: + print(f"Join missing from query: {query}", file=sys.stderr) + self.assertTrue("NATURAL INNER JOIN" in query) + +# vim: set shiftwidth=4 softtabstop=4 expandtab: commit 2dde26dcaee48d37a508617515addce634d0c96c Author: Neil Roberts <[email protected]> AuthorDate: Mon Nov 10 17:41:46 2025 +0100 Commit: Noel Grandin <[email protected]> CommitDate: Tue Nov 11 06:46:36 2025 +0100 tdf#99619 Don’t add undo action when removing conn but not deleting Previously when undo is invoked to undo adding a connection between two tables OQueryTableView::RemoveConnection gets invoked which would end up trying to add another undo action which would re-add the connection when invoked. Because this happens during an undo invocation, the undo manager is disabled and instead the new undo action gets destroyed as soon as the first undo invocation completes. However, because the new action is a delete it tries to become the owner of the deleted connection. That means that when it’s destroyed it will call dispose on the connection. However, the original undo action also tries to become the owner of the connection even though it is now disposed so that it can put the connection back if redo is invoked on the action. In that case the first undo action would try to put back a connection which is already disposed which ultimately leads to a crash. In order to fix this, the RemoveConnection implementation now avoids adding the undo action if the bDelete parameter is false. As far as I can tell from inspecting the code the only way this can happen is if RemoveConnection is invoked from OQueryTableView::DropConnection and that only happens when applying the undo/redo actions. I think it semantically makes sense not to make an undo action for the removal when bDelete is false because the undo action assumes the connection is either currently linked to a table or it itself is the owner, which means that it will ultimately cause the connection to be deleted. If the connection being removed shouldn’t be deleted then it implies there is already another owner. Change-Id: I69976c7fa8f035f9d1613382e7d0698311cf4b71 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193761 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins diff --git a/dbaccess/source/ui/querydesign/QueryTableView.cxx b/dbaccess/source/ui/querydesign/QueryTableView.cxx index 3ddb54f5986d..5a3f83894303 100644 --- a/dbaccess/source/ui/querydesign/QueryTableView.cxx +++ b/dbaccess/source/ui/querydesign/QueryTableView.cxx @@ -606,18 +606,22 @@ void OQueryTableView::createNewConnection() SelectConn( pConn ); } -bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, bool /*_bDelete*/) +bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, bool bDelete) { VclPtr<OQueryTableConnection> xConnection(static_cast<OQueryTableConnection*>(rConnection.get())); // we don't want that our connection will be deleted, we put it in the undo manager bool bRet = OJoinTableView::RemoveConnection(rConnection, false); - // add undo action - addUndoAction(this, - std::make_unique<OQueryDelTabConnUndoAction>(this), - xConnection.get(), - true); + // Don’t add an undo action if the connection isn’t to be deleted because the action will + // effectively take ownership of it and end up deleting it. See tdf#99619 + if (bDelete) + { + addUndoAction(this, + std::make_unique<OQueryDelTabConnUndoAction>(this), + xConnection.get(), + true); + } return bRet; }
