Clean gui info files

cylc/uiserver/scripts/gui.py

@@ -26,10 +26,11 @@
 from contextlib import suppress
 from glob import glob
 import os
-from pathlib import Path
-import random
 import re
+from requests.exceptions import RequestException
+import requests
 import sys
+from typing import Optional
 import webbrowser
 
 
@@ -53,25 +54,78 @@ def main(*argv):
     jp_server_opts, new_gui, workflow_id = parse_args_opts()
     if '--help' not in sys.argv:
         # get existing jpserver-<pid>-open.html files
-        # assume if this exists then the server is still running
-        # these files are cleaned by jpserver on shutdown
+        # check if the server is available for use
+        # prompt for user whether to clean files for un-usable uiservers
+        # these files are usually cleaned by jpserver on shutdown, although
+        # can be left behind on crash or a `kill -9` of the process
         existing_guis = glob(os.path.join(INFO_FILES_DIR, "*open.html"))
         if existing_guis and not new_gui:
-            gui_file = random.choice(existing_guis)
-            print(
-                "Opening with existing gui." +
-                f" Use {CLI_OPT_NEW} option for a new gui.",
-                file=sys.stderr
-            )
-            update_html_file(gui_file, workflow_id)
-            if '--no-browser' not in sys.argv:
-                webbrowser.open(f'file://{gui_file}', autoraise=True)
-            return
+            url = select_info_file(existing_guis)
+            if url:
+                print(
+                    "Opening with existing gui." +
+                    f" Use {CLI_OPT_NEW} option for a new gui.",
+                    file=sys.stderr
+                )
+                url = update_url(url, workflow_id)
+                if '--no-browser' not in sys.argv:
+                    webbrowser.open(url, autoraise=True)
+                return
     return CylcUIServer.launch_instance(
         jp_server_opts or None, workflow_id=workflow_id
     )
 
 
+def select_info_file(existing_guis: list) -> Optional[str]:
+    """This will select an active ui-server info file"""
+    existing_guis.sort(key=os.path.getmtime, reverse=True)
+    for gui_file in existing_guis:
+        url = fish_url_from_file(gui_file)
+        if url and is_active_gui(url):
+            return url
+        check_remove_file(gui_file)
+    return None
+
+
+def fish_url_from_file(gui_file):
+    with open(gui_file, "r") as f:
+        file_content = f.read()
+    url_extract_regex = re.compile('url=(.*?)\"')
+    match = url_extract_regex.search(file_content)
+    return match.group(1) if match else None
+
+
+def is_active_gui(url):
+    """Returns true if return code is 200 from server"""
+    try:
+        req = requests.get(url)
+        if req.status_code == 200:
+            return True
+    except RequestException:
+        return False
+
+
+def clean_info_files(gui_file):
+    pid = re.compile(r'-(\d*)-open\.html').search(gui_file).group(1)
+    json_file = os.path.join(INFO_FILES_DIR, f"jpserver-{pid}.json")
+    try:
+        os.unlink(gui_file)
+        os.unlink(json_file)
+    except Exception:
+        pass
+
+
+def check_remove_file(gui_file) -> None:
+    """Ask user if they want to remove the file."""
+    print("The following file cannot be used to open the Cylc GUI:"
+          f" {gui_file}.\nThe ui-server may be running on another host,"
+          " or it may be down.")
+    ret = input('Do you want to remove this file? (y/n): ')
+    if ret.lower() == 'y':
+        clean_info_files(gui_file)
+    return
+
+
 def print_error(error: str, msg: str):
     """Print formatted error with message"""
     print(cparse(
@@ -135,43 +189,29 @@ def get_arg_parser():
     return parser
 
 
-def update_html_file(gui_file, workflow_id):
-    """ Update the html file to open at the correct workflow in the gui.
+def update_url(url, workflow_id):
+    """ Update the url to open at the correct workflow in the gui.
     """
-    with open(gui_file, "r") as f:
-        file_content = f.read()
-    url_extract_regex = re.compile('url=(.*?)\"')
-    url_string = url_extract_regex.search(file_content)
-    if not url_string:
+    if not url:
         return
-    url = url_string.group(1)
     split_url = url.split('/workspace/')
     if not workflow_id:
         # new url should point to dashboard
         if len(split_url) == 1:
             # no update required
-            return
+            return url
         else:
             # previous url points to workflow page and needs updating
             # replace with base url (including token)
-            replacement_url_string = split_url[0]
+            return split_url[0]
     else:
         if len(split_url) > 1:
             old_workflow = split_url[1]
             if workflow_id == old_workflow:
                 # same workflow page requested, no update needed
-                return
+                return url
             else:
-                replacement_url_string = url.replace(old_workflow, workflow_id)
+                return url.replace(old_workflow, workflow_id)
         else:
             # current url points to dashboard, update to point to workflow
-            replacement_url_string = f"{url}/workspace/{workflow_id}"
-    update_url_string(gui_file, url, replacement_url_string)
-
-
-def update_url_string(gui_file: str, url: str, replacement_url_string: str):
-    """Updates the url string in the given gui file."""
-    file = Path(gui_file)
-    current_text = file.read_text()
-    updated_text = current_text.replace(url, replacement_url_string)
-    file.write_text(updated_text)
+            return f"{url}/workspace/{workflow_id}"

cylc/uiserver/tests/test_gui.py

@@ -13,59 +13,153 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import json
+import builtins
+from glob import glob
+import os
 from pathlib import Path
 import pytest
-import os
+from random import randint
+import requests
+from shutil import rmtree
+from time import sleep
 
-from cylc.uiserver.scripts.gui import update_html_file
+from cylc.uiserver.scripts.gui import (
+    fish_url_from_file,
+    select_info_file,
+    update_url
+)
 
 @pytest.mark.parametrize(
     'existing_content,workflow_id,expected_updated_content',
     [
         pytest.param(
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#',
             None,
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#',
             id='existing_no_workflow_new_no_workflow'
         ),
         pytest.param(
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#',
             'some/workflow',
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow',
             id='existing_no_workflow_new_workflow'
         ),
         pytest.param(
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow',
             'another/flow',
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/another/flow" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/another/flow',
             id='existing_workflow_new_workflow'
         ),
         pytest.param(
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow',
             None,
-            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#" /> ',
+            'http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#',
             id='existing_workflow_no_new_workflow'
         ),
         pytest.param(
-            'content="1;no url in this file "',
+            '',
             'another/flow',
-            'content="1;no url in this file "',
+            None,
             id='no_url_no_change'
         ),
     ]
 )
 def test_update_html_file_updates_gui_file(
     existing_content,
     workflow_id,
-    expected_updated_content,
-    tmp_path):
-    """Tests html file is updated correctly"""
+        expected_updated_content):
+    """Tests url is updated correctly"""
+
+    updated_file_content = update_url(existing_content, workflow_id)
+    assert updated_file_content == expected_updated_content
+
+
+@pytest.mark.parametrize(
+    'file_content,expected_url',
+    [
+        pytest.param(
+            'content="1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow" /> ',
+            "http://localhost:8892/cylc/?token=1234567890some_big_long_token1234567890#/workspace/some/workflow",
+            id='url_in_file'
+        ),
+        pytest.param(
+            'There is no url in here',
+            None,
+            id='no_url_in_file'
+        ),
+    ]
+)
+def test_fish_url_from_file(file_content, expected_url, tmp_path):
     Path(tmp_path).mkdir(exist_ok=True)
     tmp_gui_file = Path(tmp_path / "gui")
     tmp_gui_file.touch()
-    tmp_gui_file.write_text(existing_content)
-    update_html_file(tmp_gui_file, workflow_id)
-    updated_file_content = tmp_gui_file.read_text()
+    tmp_gui_file.write_text(file_content)
+    actual_url = fish_url_from_file(tmp_gui_file)
+    assert actual_url == expected_url
+    rmtree(tmp_path, ignore_errors=True)
 
-    assert updated_file_content == expected_updated_content
+
+def test_gui_selection_and_clean_process(tmp_path, monkeypatch):
+    """Testing functionally the gui selection and cleaning process"""
+    # set up file structure
+    info_files_dir = Path(tmp_path/'.cylc'/'uiserver'/'info_files')
+    info_files_dir.mkdir(parents=True, exist_ok=True)
+    for i in range(1, 5):
+        pid = randint(1000, 100000)
+        html_file = (info_files_dir / f"jpserver-{pid}-open.html")
+        # the json file is unused but created empty to ensure the html is the
+        # file used for selection
+        _json_file = (info_files_dir / f"jpserver-{pid}.json")
+        html_file.touch()
+        html_file.write_text(f"content=\"1;url=http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow\" more content")
+        # Sleep ensure different modification time for sort
+        sleep(0.1)
+    mock_existing_guis = glob(os.path.join(info_files_dir, "*open.html"))
+    monkeypatch.setattr(requests, 'get', mock_get)
+    url = select_info_file(mock_existing_guis)
+    # Test that the most recent ui-server is selected:
+    assert url == f"http://localhost:8892/cylc/?token=1234567890some_big_long_token{pid}#/workspace/some/workflow"
+    rmtree(tmp_path, ignore_errors=True)
+
+
+def test_cleaning_of_info_files(tmp_path, monkeypatch):
+    """Functionally tests the cleaning logic of the info files"""
+    mock_info_files_dir = Path(tmp_path/'.cylc'/'uiserver'/'info_files')
+    mock_info_files_dir.mkdir(parents=True, exist_ok=True)
+    html_file = (mock_info_files_dir / f"jpserver-12345-open.html")
+    json_file = (mock_info_files_dir / f"jpserver-12345.json")
+    json_file.touch()
+    html_file.touch()
+    html_file.write_text(f"Some content but no url in here")
+    assert html_file.exists() is True
+    assert json_file.exists() is True
+    # assert is called
+    mock_existing_guis = glob(os.path.join(mock_info_files_dir, "*open.html"))
+    monkeypatch.setattr(builtins, 'input', lambda *args, **kwargs: 'n')
+    # test that a no user response keeps the files
+    url = select_info_file(mock_existing_guis)
+    assert url is None
+    assert html_file.exists() is True
+    assert json_file.exists() is True
+    # Change user response to a yes and test files are removed
+    monkeypatch.setattr(builtins, 'input', lambda *args, **kwargs: 'y')
+    monkeypatch.setattr(
+        'cylc.uiserver.scripts.gui.INFO_FILES_DIR',
+        mock_info_files_dir
+    )
+    url = select_info_file(mock_existing_guis)
+    assert url is None
+    # test clean takes place
+    assert html_file.exists() is False
+    assert json_file.exists() is False
+    rmtree(tmp_path, ignore_errors=True)
+
+
+class MockResponse:
+    """Used for the response of mocked request"""
+    def __init__(self):
+        self.status_code = 200
+
+
+def mock_get(*args, **kwargs):
+    return MockResponse()

setup.cfg

@@ -99,6 +99,7 @@ tests =
     pytest-tornasync>=0.5.0
     pytest>=6
     types-pkg_resources>=0.1.2
+    types-requests>2
 all =
     %(hub)s
     %(tests)s