From 781476858e14bfb25cce142af891c42c2cc5e8db Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 11 Mar 2024 13:20:00 +0100 Subject: [PATCH 1/3] Don't create unnecessary histories for anon users The previous galaxy_session.user guard meant that anon users would always get a new history. --- lib/galaxy/webapps/base/webapp.py | 47 ++++++++++++++++--------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index a13059484f94..fa02d840d532 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -912,29 +912,30 @@ def get_or_create_default_history(self): Gets or creates a default history and associates it with the current session. """ - - # There must be a user to fetch a default history. - if not self.galaxy_session.user: - return self.new_history() - - # Look for default history that (a) has default name + is not deleted and - # (b) has no datasets. If suitable history found, use it; otherwise, create - # new history. - stmt = select(self.app.model.History).filter_by( - user=self.galaxy_session.user, name=self.app.model.History.default_name, deleted=False - ) - unnamed_histories = self.sa_session.scalars(stmt) - default_history = None - for history in unnamed_histories: - if history.empty: - # Found suitable default history. - default_history = history - break - - # Set or create history. - if default_history: - history = default_history - self.set_history(history) + history = self.galaxy_session.current_history + if history and not history.deleted: + return history + + user = self.galaxy_session.user + if user: + # Look for default history that (a) has default name + is not deleted and + # (b) has no datasets. If suitable history found, use it; otherwise, create + # new history. + stmt = select(self.app.model.History).filter_by( + user=user, name=self.app.model.History.default_name, deleted=False + ) + unnamed_histories = self.sa_session.scalars(stmt) + default_history = None + for history in unnamed_histories: + if history.empty: + # Found suitable default history. + default_history = history + break + + # Set or create history. + if default_history: + history = default_history + self.set_history(history) else: history = self.new_history() From 5e080223a5bf0a01c9d23c49ade2145e1fbaaf50 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 11 Mar 2024 13:23:43 +0100 Subject: [PATCH 2/3] Only create histories if client includes session cookie --- lib/galaxy/webapps/base/webapp.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index fa02d840d532..c3c1dd8bbcf2 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -650,11 +650,11 @@ def _ensure_valid_session(self, session_cookie: str, create: bool = True) -> Non galaxy_session = self.__create_new_session(prev_galaxy_session, user_for_new_session) galaxy_session_requires_flush = True self.galaxy_session = galaxy_session - if self.webapp.name == "galaxy": - self.get_or_create_default_history() self.__update_session_cookie(name=session_cookie) else: self.galaxy_session = galaxy_session + if self.webapp.name == "galaxy": + self.get_or_create_default_history() # Do we need to flush the session? if galaxy_session_requires_flush: self.sa_session.add(galaxy_session) @@ -799,10 +799,10 @@ def _associate_user_history(self, user, prev_galaxy_session=None): and not users_last_session.current_history.deleted ): history = users_last_session.current_history - elif not history: - history = self.get_history(create=True, most_recent=True) if history not in self.galaxy_session.histories: self.galaxy_session.add_history(history) + if not history: + history = self.new_history() if history.user is None: history.user = user self.galaxy_session.current_history = history From c5839eec371e7a020a4a8812039fa1bd4ce5ce4a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 12 Mar 2024 15:50:21 +0100 Subject: [PATCH 3/3] Add test for history creation logic --- lib/galaxy_test/api/test_authenticate.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/galaxy_test/api/test_authenticate.py b/lib/galaxy_test/api/test_authenticate.py index 41f437d49134..7cd375516a32 100644 --- a/lib/galaxy_test/api/test_authenticate.py +++ b/lib/galaxy_test/api/test_authenticate.py @@ -53,3 +53,25 @@ def test_tool_runner_session_cookie_handling(self): current_history_json_response.raise_for_status() current_history = current_history_json_response.json() assert current_history["contents_active"]["active"] == 1 + + def test_anon_history_creation(self): + # First request: + # We don't create any histories, just return a session cookie + response = get(self.url) + cookie = {"galaxysession": response.cookies["galaxysession"]} + # Check that we don't have any histories (API doesn't auto-create new histories) + histories_response = get( + urljoin( + self.url, + "api/histories", + ) + ) + assert not histories_response.json() + # Second request, we know client follows conventions by including cookies, + # default history is created. + get(self.url, cookies=cookie) + second_histories_response = get( + urljoin(self.url, "history/current_history_json"), + cookies=cookie, + ) + assert second_histories_response.json()