diff --git a/examples/gui_local_slot_editing.py b/examples/gui_local_slot_editing.py index 0d8a0c3..dd9d6a1 100644 --- a/examples/gui_local_slot_editing.py +++ b/examples/gui_local_slot_editing.py @@ -3,6 +3,7 @@ import json import os +import re # import yaml from pathlib import Path @@ -52,6 +53,33 @@ def str_or_none(value): return str(value) +def compute_page_storage_path( + working_dir: str, + domain_str: str, + page_name_as_filename: bool, + label_str: str = None, + top_level: str = None, +) -> str: + """Compute the folder that a page's slots are downloaded to / uploaded from. + + Mirrors the folder-naming logic previously inlined in save_as_page_package: the + page label is used when 'page_name_as_filename' is set and a label is known, + otherwise the top-level page title is used. A domain sub folder is inserted so + that pages from different OSW instances don't collide inside the same local + working directory. + + Both the '-DL-' (download) and '-UL-' (upload) event handlers call this helper + so they always agree on where a given page's content is stored on disk. + """ + if page_name_as_filename and label_str is not None: + page_dir_name = label_str + else: + page_dir_name = top_level + # Sanitize the domain for use as a directory name (invalid on Windows: \/:*?"<>|) + domain_dir = re.sub(r'[\\/:*?"<>|]', "_", domain_str) + return os.path.join(working_dir, domain_dir, page_dir_name) + + def create_config_from_setting(settings_: dict) -> WtPage.PageDumpConfig: config_ = WtPage.PageDumpConfig( target_dir=settings_["local_working_directory"], @@ -66,6 +94,7 @@ def save_as_page_package( full_page_name_str, wtsite_inst: WtSite, dump_config_inst: WtPage.PageDumpConfig, + domain_str: str, label_str: str = None, top_level: str = None, sub_level: str = None, # "content" @@ -81,10 +110,13 @@ def save_as_page_package( package_branch = "deleteme" publisher = "Open Semantic World" working_dir = dump_config_inst.target_dir - if dump_config_inst.page_name_as_filename and label_str is not None: - target_dir = os.path.join(working_dir, label_str) - else: - target_dir = os.path.join(working_dir, top_level) + target_dir = compute_page_storage_path( + working_dir, + domain_str, + dump_config_inst.page_name_as_filename, + label_str, + top_level, + ) if isinstance(author, list): author_list = author elif isinstance(author, str): @@ -156,24 +188,46 @@ def save_as_page_package( } settings_read_from_file = False - domains, accounts = read_domains_from_credentials_file(settings["cred_filepath"]) - if "wiki-dev.open-semantic-lab.org" in domains: - domain = "wiki-dev.open-semantic-lab.org" + # An empty/missing accounts.pwd.yaml must not prevent the window from opening. + # The user can still type/select a domain once the window is up. + domains, accounts, cred_load_error = [], {}, None + try: + domains, accounts = read_domains_from_credentials_file( + settings["cred_filepath"] + ) + except (FileNotFoundError, ValueError) as e: + cred_load_error = f"{type(e).__name__}: {e}" + + if domains: + if "wiki-dev.open-semantic-lab.org" in domains: + domain = "wiki-dev.open-semantic-lab.org" + else: + domain = domains[0] + if settings_read_from_file: + settings["domain"] = domain else: - domain = domains[0] - if settings_read_from_file: - settings["domain"] = domain + domain = settings.get("domain", "") cm = CredentialManager(cred_filepath=settings["cred_filepath"]) - osw_obj = OswExpress(domain=domain, cred_mngr=cm) - wtsite_obj = osw_obj.site + osw_obj = None + wtsite_obj = None + if domain: + osw_obj = OswExpress(domain=domain, cred_mngr=cm) + wtsite_obj = osw_obj.site full_page_name = settings["target_page"].split("/")[-1].replace("_", " ") - page = wtsite_obj.get_page(WtSite.GetPageParam(titles=[full_page_name])).pages[0] + page = None + if wtsite_obj is not None: + page = wtsite_obj.get_page(WtSite.GetPageParam(titles=[full_page_name])).pages[ + 0 + ] label_set = False label = None slots_downloaded = False dump_config = create_config_from_setting(settings) + label_default_text = "" + if cred_load_error is not None: + label_default_text = f"Could not load credentials: {cred_load_error}" # ----- GUI Definition ----- # Setting the theme of the GUI @@ -253,7 +307,12 @@ def save_as_page_package( ], [ # A display element that will show the label of the OSW page - psg.Multiline(size=(50, 1), key="-LABEL-", no_scrollbar=True) + psg.Multiline( + size=(50, 1), + key="-LABEL-", + no_scrollbar=True, + default_text=label_default_text, + ) ], ]), ], @@ -354,10 +413,17 @@ def save_as_page_package( json.dump(settings, f, indent=4) elif event == "-CREDENTIALS-": settings["cred_filepath"] = values["-CREDENTIALS-"] - domains, accounts = read_domains_from_credentials_file( - settings["cred_filepath"] - ) - window["-DOMAIN-"].update(values=domains) + try: + domains, accounts = read_domains_from_credentials_file( + settings["cred_filepath"] + ) + window["-DOMAIN-"].update(values=domains) + except (FileNotFoundError, ValueError) as e: + # Keep the previously loaded domains/accounts, the user can retry + # with a different file. + window["-LABEL-"].update( + f"Could not load credentials: {type(e).__name__}: {e}" + ) elif event == "-LWD-": settings["local_working_directory"] = values["-LWD-"] elif event == "-DOMAIN-": @@ -368,51 +434,82 @@ def save_as_page_package( elif event == "Load page": window["-LABEL-"].update("Loading page...") window["-DL_RES-"].update("") - full_page_name = values["-ADDRESS-"].split("/")[-1].replace("_", " ") - if (values["-ADDRESS-"].find("/wiki/") != -1) or ( - values["-ADDRESS-"].find("/w/") != -1 - ): - settings["target_page"] = values["-ADDRESS-"] - else: - settings["target_page"] = ( - "https://" + domain + "/wiki/" + full_page_name + if wtsite_obj is None: + window["-LABEL-"].update( + "No domain selected. Load a credentials file or select a " + "domain first." ) - if values["-ADDRESS-"].find(settings["domain"]) == -1: - window["-LABEL-"].update("Page not on selected domain!") label_set = False else: - # use connection - page = wtsite_obj.get_page( - WtSite.GetPageParam(titles=[full_page_name]) - ).pages[0] - if page.exists: - jsondata = page.get_slot_content("jsondata") - if jsondata is None: - window["-LABEL-"].update("Slot 'jsondata' is empty!") - label_set = False - else: - label = jsondata["label"][0]["text"] - window["-LABEL-"].update(label) - label_set = True + full_page_name = values["-ADDRESS-"].split("/")[-1].replace("_", " ") + if (values["-ADDRESS-"].find("/wiki/") != -1) or ( + values["-ADDRESS-"].find("/w/") != -1 + ): + settings["target_page"] = values["-ADDRESS-"] else: - window["-LABEL-"].update("Page does not exist!") + settings["target_page"] = ( + "https://" + domain + "/wiki/" + full_page_name + ) + if values["-ADDRESS-"].find(settings["domain"]) == -1: + window["-LABEL-"].update("Page not on selected domain!") label_set = False + else: + # use connection + page = wtsite_obj.get_page( + WtSite.GetPageParam(titles=[full_page_name]) + ).pages[0] + if page.exists: + jsondata = page.get_slot_content("jsondata") + if jsondata is None: + window["-LABEL-"].update("Slot 'jsondata' is empty!") + label_set = False + else: + label = jsondata["label"][0]["text"] + window["-LABEL-"].update(label) + label_set = True + else: + window["-LABEL-"].update("Page does not exist!") + label_set = False elif event == "-EXC_EMPTY-" or event == "-INC_EMPTY-": settings["dump_empty_slots"] = values["-INC_EMPTY-"] elif event == "-DL-": window["-DL_RES-"].update("Downloading slots...") if label_set: - dump_config = create_config_from_setting(settings) - _ = save_as_page_package( - full_page_name_str=full_page_name, - wtsite_inst=wtsite_obj, - dump_config_inst=dump_config, - label_str=label, - sub_level=SUB_LEVEL, - author=accounts[domains[0]]["username"], - ) - slots_downloaded = True - window["-DL_RES-"].update("Slots downloaded!") + try: + dump_config = create_config_from_setting(settings) + target_dir = compute_page_storage_path( + settings["local_working_directory"], + domain, + settings["page_name_as_filename"], + label, + full_page_name.split(":")[-1], + ) + window["-DL_RES-"].update( + f"Clearing '{target_dir}' and downloading slots..." + ) + # Look up the author for the selected domain, not the first + # one in the list, and degrade gracefully if there is no + # credential entry for it (e.g. empty accounts.pwd.yaml). + author = accounts.get(domain, {}).get("username") + if author is None: + window["-DL_RES-"].update( + f"No credentials found for domain '{domain}'; " + "using default author." + ) + author = "Open Semantic World" + _ = save_as_page_package( + full_page_name_str=full_page_name, + wtsite_inst=wtsite_obj, + dump_config_inst=dump_config, + domain_str=domain, + label_str=label, + sub_level=SUB_LEVEL, + author=author, + ) + slots_downloaded = True + window["-DL_RES-"].update("Slots downloaded!") + except Exception as e: + window["-DL_RES-"].update(f"{type(e).__name__}: {e}") else: window["-DL_RES-"].update("No page loaded!") elif event == "-UL_SEL-": @@ -434,29 +531,45 @@ def save_as_page_package( elif len(slots_to_upload) == 0: window["-UL_RES-"].update("No slots selected!") else: - if not slots_downloaded: - window["-UL_RES-"].update("No slots downloaded!") - dump_config = create_config_from_setting(settings) - window["-UL_RES-"].update("Uploading slots...") - if SUB_LEVEL is None: - storage_path = Path(dump_config.target_dir) - else: - storage_path = Path(dump_config.target_dir).parent - pages = wtsite_obj.read_page_package( - WtSite.ReadPagePackageParam( - storage_path=storage_path, - packages_info_file_name=PACKAGE_INFO_FILE_NAME, - selected_slots=slots_to_upload, - debug=False, + try: + # Recompute the storage path the same way the download path + # does, instead of trusting a possibly stale dump_config. + # This also makes upload work against a folder written by a + # previous run, even if no download happened this session. + storage_path = Path( + compute_page_storage_path( + settings["local_working_directory"], + domain, + settings["page_name_as_filename"], + label, + full_page_name.split(":")[-1], + ) ) - ).pages - param = wtsite_obj.UploadPageParam(pages=pages, parallel=False) - wtsite_obj.upload_page(param) - # Success: - window["-UL_RES-"].update("Slots uploaded!") - # Report in the download section that the slots have been uploaded to - # remind the user that he eventually has to re-download the slots - window["-DL_RES-"].update("Slots uploaded!") + if not slots_downloaded: + window["-UL_RES-"].update( + "No slots downloaded this session, trying folder from a " + f"previous run at '{storage_path}'..." + ) + else: + window["-UL_RES-"].update("Uploading slots...") + pages = wtsite_obj.read_page_package( + WtSite.ReadPagePackageParam( + storage_path=storage_path, + packages_info_file_name=PACKAGE_INFO_FILE_NAME, + selected_slots=slots_to_upload, + debug=False, + ) + ).pages + param = wtsite_obj.UploadPageParam(pages=pages, parallel=False) + wtsite_obj.upload_page(param) + # Success: + window["-UL_RES-"].update("Slots uploaded!") + # Report in the download section that the slots have been + # uploaded to remind the user that he eventually has to + # re-download the slots + window["-DL_RES-"].update("Slots uploaded!") + except Exception as e: + window["-UL_RES-"].update(f"{type(e).__name__}: {e}") # Some debugging output functionality if DEBUG: diff --git a/pyproject.toml b/pyproject.toml index 8a8ccf5..5702993 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,7 +68,11 @@ dataimport = [ "deepl", "openpyxl", ] -UI = ["pysimplegui"] +UI = [ + # PySimpleGUI 5 was commercial and its releases are yanked from PyPI. + # The LGPLv3 line resumed at version 6, which the GUI examples target. + "pysimplegui>=6", +] workflow = [ "prefect>=2.20.25,<3.0", # prefect 2.20.25 is the final 2.x release (no backports). Its diff --git a/src/osw/model/page_package.py b/src/osw/model/page_package.py index f9e425b..a29061a 100644 --- a/src/osw/model/page_package.py +++ b/src/osw/model/page_package.py @@ -171,6 +171,10 @@ class PagePackageConfig(BaseModel): content_path: Optional[Union[str, Path]] = "" """The directory where the content (pages, files) is stored. Will be created automatically if not existing.""" + clear_content_dir: Optional[bool] = True + """If True (default), the content directory is deleted before the page + package is created. Set to False to keep any existing content, e.g. when + reusing a directory that already holds locally edited slot files.""" titles: List[str] """List of page titles.""" ignore_titles: Optional[List[str]] = None diff --git a/src/osw/wiki_tools.py b/src/osw/wiki_tools.py index 1195e74..7139ee2 100644 --- a/src/osw/wiki_tools.py +++ b/src/osw/wiki_tools.py @@ -48,6 +48,10 @@ def read_domains_from_credentials_file( with open(cred_filepath, encoding="utf-8") as stream_: try: accounts_dict = yaml.safe_load(stream_) + # An empty file is parsed as None by yaml.safe_load, which would + # otherwise raise an AttributeError on the .keys() call below + if accounts_dict is None: + accounts_dict = {} domains_list = list(accounts_dict.keys()) if len(domains_list) == 0: raise ValueError("No domain found in accounts.pwd.yaml!") diff --git a/src/osw/wtsite.py b/src/osw/wtsite.py index 37dc493..8193c3e 100644 --- a/src/osw/wtsite.py +++ b/src/osw/wtsite.py @@ -885,14 +885,15 @@ def create_page_package(self, param: CreatePagePackageParam): config = param.config # Clear the content directory - try: - if debug: - print(f"Delete dir '{config.content_path}'") - if os.path.exists(config.content_path): - shutil.rmtree(config.content_path) - except OSError as e: - if debug: - print(f"Error: {e.filename} - {e.strerror}.") + if config.clear_content_dir: + try: + if debug: + print(f"Delete dir '{config.content_path}'") + if os.path.exists(config.content_path): + shutil.rmtree(config.content_path) + except OSError as e: + if debug: + print(f"Error: {e.filename} - {e.strerror}.") # Create a dump config if dump_config is None: dump_config = WtPage.PageDumpConfig( @@ -1096,7 +1097,12 @@ def read_page_package(self, param: ReadPagePackageParam) -> ReadPagePackageResul ) # Read packages info file with open(pi_fp, encoding="utf-8") as f: - packages_json = json.load(f) + try: + packages_json = json.load(f) + except json.JSONDecodeError as e: + raise json.JSONDecodeError( + f"Malformed JSON in '{pi_fp}': {e.msg}", e.doc, e.pos + ) from e # Assume that the pages files are located in the subdir storage_path_content = ut.list_files_and_directories( search_path=storage_path, recursive=True @@ -1130,7 +1136,14 @@ def get_slot_content( if len(file_content) > 0: if url_path.endswith(".json"): with open(slot_path, encoding="utf-8") as f: - slot_data = json.load(f) + try: + slot_data = json.load(f) + except json.JSONDecodeError as e: + raise json.JSONDecodeError( + f"Malformed JSON in '{slot_path}': {e.msg}", + e.doc, + e.pos, + ) from e return slot_data elif url_path.endswith(".wikitext"): slot_data = file_content diff --git a/tests/test_wiki_tools.py b/tests/test_wiki_tools.py index 2d73cca..f1c2e4f 100644 --- a/tests/test_wiki_tools.py +++ b/tests/test_wiki_tools.py @@ -13,6 +13,41 @@ def test_create_flat_content_structure_from_wikitext(): assert result == expected +def test_read_domains_from_credentials_file_empty_file_raises_value_error(tmp_path): + """An empty file is parsed as None by yaml.safe_load. Before the fix this + raised an unhandled AttributeError from accounts_dict.keys(); it must now + raise the same clear ValueError as an empty mapping.""" + cred_file = tmp_path / "accounts.pwd.yaml" + cred_file.write_text("", encoding="utf-8") + + with pytest.raises(ValueError, match="No domain found"): + wt.read_domains_from_credentials_file(cred_file) + + +def test_read_domains_from_credentials_file_empty_mapping_raises_value_error( + tmp_path, +): + cred_file = tmp_path / "accounts.pwd.yaml" + cred_file.write_text("{}", encoding="utf-8") + + with pytest.raises(ValueError, match="No domain found"): + wt.read_domains_from_credentials_file(cred_file) + + +def test_read_domains_from_credentials_file_valid_file_returns_domains_and_accounts( + tmp_path, +): + cred_file = tmp_path / "accounts.pwd.yaml" + cred_file.write_text( + "example.org:\n username: user\n password: pass\n", encoding="utf-8" + ) + + domains, accounts = wt.read_domains_from_credentials_file(cred_file) + + assert domains == ["example.org"] + assert accounts == {"example.org": {"username": "user", "password": "pass"}} + + def _ask_result(*titles): """Build a minimal SMW ``ask`` API result dict for the given page titles.""" return { diff --git a/tests/test_wtsite_create_page_package.py b/tests/test_wtsite_create_page_package.py new file mode 100644 index 0000000..d2d3c02 --- /dev/null +++ b/tests/test_wtsite_create_page_package.py @@ -0,0 +1,86 @@ +"""Unit tests for the PagePackageConfig.clear_content_dir purge flag. + +Regression guard for #42: WtSite.create_page_package used to unconditionally +shutil.rmtree() the content directory. clear_content_dir defaults to True (no +behaviour change), but setting it to False must keep any existing content. +""" + +import threading + +import pytest + +import osw.model.page_package as package +from osw.wtsite import WtPage, WtSite + + +class _FakeSite: + """Stands in for mwclient.Site, only used if a re-login is attempted.""" + + host = "example.org" + + +def _make_fake_wtsite(): + """A WtSite that performs no network calls.""" + ws = WtSite.__new__(WtSite) + ws._session_lock = threading.RLock() + ws._site = _FakeSite() + return ws + + +def _make_config(tmp_path, clear_content_dir): + bundle = package.PagePackageBundle( + packages={ + "TestPkg": package.PagePackage( + globalID="org.test.TestPkg", + description="test package", + version="0.0.1", + baseURL="https://example.org/", + ) + } + ) + return package.PagePackageConfig( + name="TestPkg", + config_path=tmp_path / "packages.json", + content_path=tmp_path / "content", + bundle=bundle, + titles=["Item:OSW123"], + include_files=False, + clear_content_dir=clear_content_dir, + ) + + +def test_clear_content_dir_defaults_to_true(tmp_path): + bundle = package.PagePackageBundle(packages={}) + config = package.PagePackageConfig( + name="TestPkg", + config_path=tmp_path / "packages.json", + titles=["Item:OSW123"], + bundle=bundle, + ) + assert config.clear_content_dir is True + + +@pytest.mark.parametrize("clear_content_dir", [True, False]) +def test_create_page_package_honours_clear_content_dir(tmp_path, clear_content_dir): + ws = _make_fake_wtsite() + page = WtPage(wtSite=ws, title="Item:OSW123", do_init=False) + + content_path = tmp_path / "content" + content_path.mkdir() + marker = content_path / "marker.txt" + marker.write_text("pre-existing content", encoding="utf-8") + + config = _make_config(tmp_path, clear_content_dir=clear_content_dir) + + ws.create_page_package( + WtSite.CreatePagePackageParam( + config=config, + offline_pages={"Item:OSW123": page}, + debug=False, + ) + ) + + if clear_content_dir: + assert not marker.exists() + else: + assert marker.exists() diff --git a/tests/test_wtsite_read_page_package.py b/tests/test_wtsite_read_page_package.py new file mode 100644 index 0000000..928ce5d --- /dev/null +++ b/tests/test_wtsite_read_page_package.py @@ -0,0 +1,78 @@ +"""Unit tests for WtSite.read_page_package()'s JSON error handling. + +Regression guard for #42: a malformed packages.json or slot file used to raise a +bare json.JSONDecodeError with no indication of which file was broken. Both +json.load() call sites must now name the offending file path. +""" + +import json +import threading + +import pytest + +from osw.wtsite import WtSite + + +class _FakeSite: + """Stands in for mwclient.Site, only used if a re-login is attempted.""" + + host = "example.org" + + +def _make_fake_wtsite(): + """A WtSite that performs no network calls.""" + ws = WtSite.__new__(WtSite) + ws._session_lock = threading.RLock() + ws._site = _FakeSite() + return ws + + +def _valid_packages_json(): + return { + "packages": { + "TestPkg": { + "globalID": "org.test.TestPkg", + "description": "test package", + "version": "0.0.1", + "baseURL": "https://example.org/", + "pages": [ + { + "name": "OSW123", + "namespace": "NS_ITEM", + "urlPath": "OSW123.wikitext", + "slots": {"jsondata": {"urlPath": "OSW123.slot_jsondata.json"}}, + } + ], + } + } + } + + +def test_read_page_package_names_malformed_packages_json(tmp_path): + packages_json_path = tmp_path / "packages.json" + packages_json_path.write_text("{not valid json", encoding="utf-8") + + ws = _make_fake_wtsite() + + with pytest.raises(json.JSONDecodeError) as exc_info: + ws.read_page_package(WtSite.ReadPagePackageParam(storage_path=tmp_path)) + + assert str(packages_json_path) in str(exc_info.value) + assert exc_info.value.__cause__ is not None + + +def test_read_page_package_names_malformed_slot_file(tmp_path): + packages_json_path = tmp_path / "packages.json" + packages_json_path.write_text(json.dumps(_valid_packages_json()), encoding="utf-8") + slot_path = tmp_path / "OSW123.slot_jsondata.json" + slot_path.write_text("{not valid json", encoding="utf-8") + # The main slot content file also needs to exist for the dump to be read. + (tmp_path / "OSW123.wikitext").write_text("some wikitext", encoding="utf-8") + + ws = _make_fake_wtsite() + + with pytest.raises(json.JSONDecodeError) as exc_info: + ws.read_page_package(WtSite.ReadPagePackageParam(storage_path=tmp_path)) + + assert str(slot_path) in str(exc_info.value) + assert exc_info.value.__cause__ is not None diff --git a/uv.lock b/uv.lock index 516050d..294371f 100644 --- a/uv.lock +++ b/uv.lock @@ -2098,7 +2098,7 @@ requires-dist = [ { name = "pybars3-wheel" }, { name = "pydantic", extras = ["email"], specifier = ">=2.12.0" }, { name = "pyld" }, - { name = "pysimplegui", marker = "extra == 'ui'" }, + { name = "pysimplegui", marker = "extra == 'ui'", specifier = ">=6" }, { name = "pyyaml" }, { name = "rdflib" }, { name = "requests" }, @@ -2610,10 +2610,11 @@ wheels = [ [[package]] name = "pysimplegui" -version = "6.2" +version = "6.3.0.1" source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/99/58/bb2d530bfc597e1a86378b508a96bc57ff588af7b37ebd68fb4eec62a871/pysimplegui-6.3.0.1.tar.gz", hash = "sha256:d711ac856a8c03382ecc13e58468c8d6b02c580881fc4115c770691b0db680ea", size = 498434, upload-time = "2026-08-16T19:09:14.019Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/35/6d/62c502ed8cdbd13588b492f27c8728933c95be144bcbc431c1cb0d83947a/pysimplegui-6.2-py3-none-any.whl", hash = "sha256:6e60bd390b370bfdf35a2507576317a93674cfacd52d21d07c5a40867e14779f", size = 487911, upload-time = "2026-06-17T13:42:43.377Z" }, + { url = "https://files.pythonhosted.org/packages/45/69/dcff3a6f9c8b411b967a56cf34eafac2d50a14d5ea4efa15e2aa774c28cf/pysimplegui-6.3.0.1-py3-none-any.whl", hash = "sha256:937da4ae6dd6a93b8239aefc76f717e1f4861d4fb64cd4e9b3a5df88865520a5", size = 494935, upload-time = "2026-08-16T19:09:12.644Z" }, ] [[package]]