Compare commits

..

15 Commits

Author SHA1 Message Date
Martin Tranberg
7fd69a9c3f fix: task 10 + review fix #1 — thread safety and sleep bug
Fix #1: Don't sleep after the final exhausted retry attempt in
_graph_request(). The sleep on the last iteration was pure overhead —
the loop exits immediately after, so the caller just waited an extra
backoff period for no reason. Guard with attempt < _MAX_RETRIES - 1.

Task 10: Add threading.Lock (_edits_lock) for all compound operations
on active_edits, which is accessed from both the UI thread and
background edit threads:
- __init__: declare self._edits_lock = threading.Lock()
- open_file: snapshot already_editing and at_limit under the lock,
  then release before showing blocking UI dialogs
- process_file: wrap initial dict assignment, waiting=True, the
  in-check+waiting=False, and in-check+del under the lock
- on_done_editing_clicked: lock the items() snapshot used to build
  waiting_files, preventing iteration over a dict being mutated

Add 8 new unit tests (1 for Fix #1 sleep count, 7 for Task 10 lock).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:36:29 +02:00
Martin Tranberg
0dfef3e611 feat: task 9 — add _graph_request helper with retry on 429/503
Add module-level _graph_request() that wraps requests.request() with:
- Up to 3 retries on HTTP 429 (rate limited) and 503 (unavailable)
- Exponential backoff capped at 60 s, honouring Retry-After header
- Default timeout=30 s injected via setdefault (caller can override)

Wire all 13 retry-eligible API calls through _graph_request(). The 3
file-upload requests.put(data=f) calls are kept direct since an open
stream cannot be re-read after the first attempt.

Add 9 unit tests covering: success path, 429/503 retry, Retry-After
header, max-retry exhaustion, timeout injection and override.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:29:39 +02:00
Martin Tranberg
707645ab36 fix: task 8 — add timeout to all requests calls
Add timeout= to every requests.get/post/put/patch/delete call so that
background threads cannot hang indefinitely when the network is stalled:

- timeout=30 on all API calls (delete, post, patch, get — 13 locations)
- timeout=120 on file upload calls (requests.put with data= — 3 locations)
  to allow sufficient time for large file transfers

Add 1 new unit test that scans the source file and fails if any
requests.* call is missing a timeout= parameter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:26:04 +02:00
Martin Tranberg
df55660291 fix: task 7 — production hardening quick fixes
- Replace 4 bare `except:` with `except Exception:` (load_settings,
  get_txt, get_icon_idx_for_file, process_file cleanup block) so
  SystemExit and KeyboardInterrupt are no longer swallowed
- Replace 2 print() calls with logger.error() (__init__ MSAL init,
  ensure_valid_token) so errors appear in the configurable log output
- Sanitize item['name'] with os.path.basename() in on_download_clicked
  and _download_folder_recursive_sync to prevent path traversal from
  server-controlled filenames
- Add 8 new unit tests covering all Task 7 changes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:22:54 +02:00
Martin Tranberg
e8e1d8b60d docs: update README with latest features and fixes
- Add pagination / large-tenant support section
- Add status bar gauge feature
- Document tabbed settings dialog with fane overview
- Add data-driven breadcrumb navigation note
- Add stale-result protection (_nav_gen) to architecture section
- Add unit test run instructions
- Update build instructions to use the spec file
- Fix typo "Star op" → "Start op"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:11:09 +02:00
Martin Tranberg
ba3d8dded5 chore: ignore tasks/ and .claude/ directories
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 10:00:52 +02:00
Martin Tranberg
d7c0fdd091 test: add unit tests for all code-review bug fixes
19 tests covering:
- I2/I3: STRINGS dict entries (System tab label, status_loading_items)
- I1/C-1/S-2: nav_gen guard logic in _finalize_list_loading
  (matching gen applies, stale gen discards, None bypasses guard,
   old zero default now correctly treated as stale)
- C1: url=None initialization order in _fetch_tree_children_bg
- S2: dead SITE branch absent from _append_list_items
- S-1: is_breadcrumb parameter removed from _navigate_to_item_data

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:57:44 +02:00
Martin Tranberg
f168d0c60a refactor: remove dead is_breadcrumb parameter from _navigate_to_item_data (S-1)
The S3 fix removed the only conditional that read is_breadcrumb. Remove
the parameter from the signature and its kwarg from the one call site in
the breadcrumb button handler.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:53:58 +02:00
Martin Tranberg
0cbec6477f fix: restore refresh operations broken by nav_gen regression (C-1, S-2)
Change nav_gen default from 0 to None in _fetch_list_contents_bg and
_finalize_list_loading. The guard is updated to only apply when a gen
is explicitly provided (`nav_gen is not None`).

Refresh call sites (lines 1515, 1529, 1538) pass no gen, so they receive
None and bypass the guard — their results are always applied. Navigation
calls still pass an explicit integer gen, so stale-navigation protection
remains fully intact.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:52:42 +02:00
Martin Tranberg
a55365259c refactor: remove redundant pulse_gauge calls and fix breadcrumb dedup (S1, S3)
- S1: drop pulse_gauge(True) from inside pagination while-loops in
  _fetch_sites_bg, _fetch_tree_children_bg, and _fetch_list_contents_bg;
  the gauge is already running from the call before the loop
- S3: remove the is_breadcrumb bypass on the early-return guard so
  clicking the already-active breadcrumb segment no longer fires a
  redundant network request

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:45:49 +02:00
Martin Tranberg
d529568798 fix: add navigation generation counter to prevent stale list overwrite (I1)
Introduces self._nav_gen, incremented on every _navigate_to_item_data
call. The counter is threaded through _fetch_list_contents_bg and
checked in _finalize_list_loading: if the user navigated away while a
fetch was in flight, the stale results are silently discarded instead of
overwriting the active folder view and re-sorting its items.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:44:39 +02:00
Martin Tranberg
b800f0308d fix: resolve C2/I4 review findings
- C2: remove duplicate EVT_SIZE binding (on_status_bar_resize); merge
  gauge repositioning into the single on_resize handler
- I4: position gauge correctly on first show by updating rect inside
  pulse_gauge._do() when start=True, so no resize event is required

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:42:52 +02:00
Martin Tranberg
dadbb4d51a fix: resolve C1/I2/I3/S2 review findings
- C1: initialize url=None before conditional in _fetch_tree_children_bg
  to prevent UnboundLocalError on unexpected data types
- I2: translate System tab label via get_txt instead of hardcoded string
- I3: add status_loading_items to STRINGS (da+en) and use it in
  _fetch_list_contents_bg instead of hardcoded Danish f-string
- S2: remove unreachable SITE branch from _append_list_items

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-12 09:41:25 +02:00
Martin Tranberg
be7a78deb8 feat: update credits section with company address and contact information 2026-04-07 13:34:56 +02:00
Martin Tranberg
4abf221887 refactor: convert settings dialog to tabbed interface and add about section 2026-04-07 13:32:19 +02:00
4 changed files with 718 additions and 132 deletions

4
.gitignore vendored
View File

@@ -45,3 +45,7 @@ searchindex.json
# Temporary files # Temporary files
C:\Temp_SP\ C:\Temp_SP\
# Project tooling / AI assistant files
tasks/
.claude/

View File

@@ -6,13 +6,27 @@ En moderne Python-baseret fil-browser til Microsoft SharePoint, specielt designe
- **Ingen Sti-begrænsning (MAX_PATH):** Problemfri og pålidelig redigering uanset mappedybde i SharePoint. - **Ingen Sti-begrænsning (MAX_PATH):** Problemfri og pålidelig redigering uanset mappedybde i SharePoint.
- **Sikker Redigering & Kollision-beskyttelse:** Automatisk Check-out/Check-in og intelligent overvågning af fil-låse lokalt. - **Sikker Redigering & Kollision-beskyttelse:** Automatisk Check-out/Check-in og intelligent overvågning af fil-låse lokalt.
- **Professionel Brugerflade:** Bygget med `wxPython` (Native Windows UI) inklusiv indfødte OS-ikoner, brødkrummesti (breadcrumbs) og lazy-loading af hierarkisk træstruktur for markant hurtigere navigation. - **Professionel Brugerflade:** Bygget med `wxPython` (Native Windows UI) inklusiv indfødte OS-ikoner, data-drevet brødkrummesti (breadcrumbs) og lazy-loading af hierarkisk træstruktur for markant hurtigere navigation.
- **Paginering & Stor-tenant understøttelse:** Alle Graph API-kald følger `@odata.nextLink` automatisk, så mapper og sites med hundredvis af elementer indlæses korrekt uden tab.
- **Statuslinje med fremgangsmåler:** En integreret statusmåler (gauge) i statuslinjen giver visuelt feedback under alle netværksoperationer.
- **First-Run Setup Wizard:** Automatisk konfigurationsguide ved første opstart, der opsamler Client ID og Tenant ID (kræver ingen forudgående manuel `settings.json`). - **First-Run Setup Wizard:** Automatisk konfigurationsguide ved første opstart, der opsamler Client ID og Tenant ID (kræver ingen forudgående manuel `settings.json`).
- **Avanceret Søgning:** Hurtig global søgefunktion der bygger på et lokalt, trinvist opdateret indeks, samt understøttelse af "OG"-logik (AND logic). - **Avanceret Søgning:** Hurtig global søgefunktion der bygger på et lokalt, trinvist opdateret indeks, samt understøttelse af "OG"-logik (AND logic).
- **Fuld Fil- og Mappestyring:** Understøtter upload, sletning og omdøbning, samt visning af udvidet fil-metadata (filstørrelse, redigeringsdato). - **Fuld Fil- og Mappestyring:** Understøtter upload, sletning og omdøbning, samt visning af udvidet fil-metadata (filstørrelse, redigeringsdato).
- **Multisprog:** Indbygget og brugerstyret understøttelse af både Dansk og Engelsk-grænseflade. - **Multisprog:** Indbygget og brugerstyret understøttelse af både Dansk og Engelsk-grænseflade.
- **Multi-File Editing:** Robust understøttelse for lokalt at redigere flere forskellige filer uafhængigt af hinanden i baggrunden uden at interface fryser. - **Multi-File Editing:** Robust understøttelse for lokalt at redigere flere forskellige filer uafhængigt af hinanden i baggrunden uden at interface fryser.
## ⚙️ Indstillinger
Indstillingsdialogen er organiseret i faner:
| Fane | Indhold |
|------|---------|
| **Konto** | Azure Client ID og Tenant ID |
| **Stier** | Midlertidig downloadmappe og app-placering |
| **Licens** | Licensnøgle og aktiveringsstatus |
| **System** | Sprog og log-output til fejlfinding |
| **Om** | Versionsinformation og kontaktoplysninger |
## 🛠️ Teknologier ## 🛠️ Teknologier
- **Sprog:** Python 3.x - **Sprog:** Python 3.x
@@ -31,7 +45,7 @@ pip install wxPython msal requests
``` ```
### Kør applikationen ### Kør applikationen
Star op med: Start op med:
```bash ```bash
python sharepoint_browser.py python sharepoint_browser.py
``` ```
@@ -39,21 +53,34 @@ python sharepoint_browser.py
Ved første kørsel uden en konfiguration vil applikationen præsentere en Setup Wizard, hvor man nemt kan indtaste Microsoft-loginoplysningerne. Indtastningerne gemmes i en lokal `settings.json` fil. Ved første kørsel uden en konfiguration vil applikationen præsentere en Setup Wizard, hvor man nemt kan indtaste Microsoft-loginoplysningerne. Indtastningerne gemmes i en lokal `settings.json` fil.
## 🏗️ Byg til EXE (Valgfrit) ## 🏗️ Byg til EXE (Valgfrit)
For at pakke programmet til en uafhængig, "kør-bar" `.exe` fil til Windows, kan dette gøres med PyInstaller: For at pakke programmet til en uafhængig, kørbar `.exe` fil til Windows bruges den medfølgende PyInstaller spec-fil:
```bash ```bash
pip install pyinstaller pip install pyinstaller
python -m PyInstaller --windowed --onefile --icon=icon.ico --name "SharePoint Browser" sharepoint_browser.py python -m PyInstaller "SharePoint Explorer.spec" --noconfirm
``` ```
*(Husk at afhæningheder og ikoner skal inddrages formelt i dit build afhængigt af din PyInstaller spec-fil).*
Den færdige fil placeres i `dist/SharePoint Explorer.exe` med ikon indlejret.
## 🧩 Arkitektur & Workflow ## 🧩 Arkitektur & Workflow
1. **Godkendelse:** Det autentificerer brugeren via MSAL & MS Graph API. 1. **Godkendelse:** Autentificerer brugeren via MSAL & MS Graph API.
2. **Navigation:** Data hentes asynkront (lazy-loading). Alt håndteres med ID'er istedet for filstier, hvilket sikrer MAX_PATH-modstandsdygtighed. 2. **Navigation:** Data hentes asynkront (lazy-loading) og chunked — de første resultater vises straks mens resten streames ind. Alt håndteres med ID'er i stedet for filstier, hvilket sikrer MAX_PATH-modstandsdygtighed.
3. **Baggrundshåndtering af redigering:** 3. **Navigationskontekst:** Brødkrummestien er data-drevet; hvert segment gemmer det fulde navigationsobjekt, så klik på en forælder altid navigerer korrekt uden at gennemsøge træet.
4. **Stale-result beskyttelse:** En navigations-tæller (`_nav_gen`) sikrer, at svar fra afbrudte netværkskald aldrig overskriver den aktive mappevisning.
5. **Baggrundshåndtering af redigering:**
- Filer tjekkes ud (`Checkout`) direkte i SharePoint. - Filer tjekkes ud (`Checkout`) direkte i SharePoint.
- Hentes ned til det lokale drev under korte stier under C-drevet eks. `C:\Temp_SP\[MD5-Hash].[ext]`. - Hentes ned til det lokale drev under korte stier, eks. `C:\Temp_SP\[MD5-Hash].[ext]`.
- Et baggrunds-thread overvåger derefter det lokale program (fx Word) kontinuerligt via `os.rename()` tricket. - Et baggrunds-thread overvåger det lokale program (fx Word) kontinuerligt via `os.rename()` tricket.
- Når filen lukkes fra dit office-program, uploades ændringerne op til SharePoint og modtager et `Checkin`. - Når filen lukkes, uploades ændringerne til SharePoint og modtager et `Checkin`.
## 🧪 Tests
En unit-test suite dækker de kritiske logik-komponenter:
```bash
python -m unittest tests.test_review_fixes -v
```
Testene kører uden skærm/display og dækker: navigations-tæller, oversættelses-nøgler, URL-initialisering og signaturer.
## 💡 Backlog / Kommende muligheder ## 💡 Backlog / Kommende muligheder
1. Integration for håndtering af flere tenants (lejemål) 1. Integration for håndtering af flere tenants (lejemål)

View File

@@ -67,13 +67,36 @@ def load_settings():
with open(SETTINGS_FILE, 'r', encoding='utf-8') as f: with open(SETTINGS_FILE, 'r', encoding='utf-8') as f:
try: try:
return json.load(f) return json.load(f)
except: except Exception:
return default_settings return default_settings
def save_settings(new_settings): def save_settings(new_settings):
with open(SETTINGS_FILE, 'w', encoding='utf-8') as f: with open(SETTINGS_FILE, 'w', encoding='utf-8') as f:
json.dump(new_settings, f, indent=4) json.dump(new_settings, f, indent=4)
# --- GRAPH API REQUEST HELPER ---
_RETRY_STATUSES = {429, 503}
_MAX_RETRIES = 3
def _graph_request(method, url, **kwargs):
"""Thin wrapper around requests.request with retry for 429/503.
Retries up to _MAX_RETRIES times when Graph API signals rate limiting
(429) or transient unavailability (503), honouring the Retry-After header
when present. A default timeout of 30 s is injected if the caller does
not supply one. File-upload calls that pass an open stream as data=
should use requests.put() directly, since a stream cannot be re-read.
"""
kwargs.setdefault("timeout", 30)
for attempt in range(_MAX_RETRIES):
res = requests.request(method, url, **kwargs)
if res.status_code not in _RETRY_STATUSES:
return res
if attempt < _MAX_RETRIES - 1: # Don't sleep after the final attempt
retry_after = int(res.headers.get("Retry-After", 2 ** attempt))
time.sleep(min(retry_after, 60))
return res # Return last response after exhausting retries
settings = load_settings() settings = load_settings()
CLIENT_ID = settings.get("client_id") CLIENT_ID = settings.get("client_id")
TENANT_ID = settings.get("tenant_id") TENANT_ID = settings.get("tenant_id")
@@ -178,7 +201,10 @@ STRINGS = {
"settings_license_key": "Licensnøgle:", "settings_license_key": "Licensnøgle:",
"settings_license_status": "Status: Ikke aktiveret", "settings_license_status": "Status: Ikke aktiveret",
"settings_logging_group": "System / Diverse", "settings_logging_group": "System / Diverse",
"settings_logging": "Aktiver log-output (anbefales til fejlfinding)" "settings_logging": "Aktiver log-output (anbefales til fejlfinding)",
"status_loading_items": "Henter... ({count} emner)",
"settings_about_group": "Om programmet",
"settings_credits": "© 2026 SharePoint Explorer\n\nSkabt af:\nMartin Tranberg\nBlueprint\n\nBernhard Bangs Allé 23, 2.\n2000 Frederiksberg\n\nTel: 70258689"
}, },
"en": { "en": {
"title": "SharePoint Explorer", "title": "SharePoint Explorer",
@@ -272,7 +298,10 @@ STRINGS = {
"settings_license_key": "License Key:", "settings_license_key": "License Key:",
"settings_license_status": "Status: Not activated", "settings_license_status": "Status: Not activated",
"settings_logging_group": "System / Miscellaneous", "settings_logging_group": "System / Miscellaneous",
"settings_logging": "Enable log output (recommended for troubleshooting)" "settings_logging": "Enable log output (recommended for troubleshooting)",
"status_loading_items": "Loading... ({count} items)",
"settings_about_group": "About",
"settings_credits": "© 2026 SharePoint Explorer\n\nCreated by:\nMartin Tranberg\nBlueprint\n\nBernhard Bangs Allé 23, 2.\nDK-2000 Frederiksberg\n\nPhone: +45 70258689"
} }
} }
@@ -403,7 +432,7 @@ class SettingsDialog(wx.Dialog):
def __init__(self, parent, current_settings): def __init__(self, parent, current_settings):
lang = current_settings.get("language", "da") lang = current_settings.get("language", "da")
title = STRINGS[lang].get("settings_title", "Settings") title = STRINGS[lang].get("settings_title", "Settings")
super().__init__(parent, title=title, size=(520, 720)) super().__init__(parent, title=title, size=(580, 550))
self.settings = current_settings.copy() self.settings = current_settings.copy()
self.lang = lang self.lang = lang
@@ -415,88 +444,101 @@ class SettingsDialog(wx.Dialog):
def InitUI(self): def InitUI(self):
vbox = wx.BoxSizer(wx.VERTICAL) vbox = wx.BoxSizer(wx.VERTICAL)
panel = wx.Panel(self) # --- TABBED INTERFACE (wx.Notebook) ---
inner_vbox = wx.BoxSizer(wx.VERTICAL) self.nb = wx.Notebook(self)
# --- Group: Authentication --- # 1. ACCOUNT TAB
auth_box = wx.StaticBox(panel, label=self.get_txt("settings_auth_group")) account_panel = wx.Panel(self.nb)
auth_sizer = wx.StaticBoxSizer(auth_box, wx.VERTICAL) account_vbox = wx.BoxSizer(wx.VERTICAL)
grid = wx.FlexGridSizer(2, 2, 10, 10) grid = wx.FlexGridSizer(2, 2, 10, 10)
grid.AddGrowableCol(1, 1) grid.AddGrowableCol(1, 1)
grid.Add(wx.StaticText(account_panel, label=self.get_txt("settings_client_id")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 5)
self.client_id_ctrl = wx.TextCtrl(account_panel, value=self.settings.get("client_id", ""), size=(-1, 25))
grid.Add(self.client_id_ctrl, 1, wx.EXPAND)
grid.Add(wx.StaticText(panel, label=self.get_txt("settings_client_id")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 5) grid.Add(wx.StaticText(account_panel, label=self.get_txt("settings_tenant_id")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 5)
self.client_id_ctrl = wx.TextCtrl(panel, value=self.settings.get("client_id", ""), size=(-1, 25)) self.tenant_id_ctrl = wx.TextCtrl(account_panel, value=self.settings.get("tenant_id", ""), size=(-1, 25))
grid.Add(self.client_id_ctrl, 1, wx.EXPAND | wx.ALIGN_CENTER_VERTICAL) grid.Add(self.tenant_id_ctrl, 1, wx.EXPAND)
grid.Add(wx.StaticText(panel, label=self.get_txt("settings_tenant_id")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 5) account_vbox.Add(grid, 1, wx.EXPAND | wx.ALL, 15)
self.tenant_id_ctrl = wx.TextCtrl(panel, value=self.settings.get("tenant_id", ""), size=(-1, 25)) account_panel.SetSizer(account_vbox)
grid.Add(self.tenant_id_ctrl, 1, wx.EXPAND | wx.ALIGN_CENTER_VERTICAL) self.nb.AddPage(account_panel, self.get_txt("settings_auth_group").split("/")[0].strip())
auth_sizer.Add(grid, 1, wx.EXPAND | wx.ALL, 10) # 2. PATHS TAB
inner_vbox.Add(auth_sizer, 0, wx.EXPAND | wx.ALL, 10) paths_panel = wx.Panel(self.nb)
paths_vbox = wx.BoxSizer(wx.VERTICAL)
paths_inner = wx.BoxSizer(wx.VERTICAL)
# --- Group: Paths --- paths_inner.Add(wx.StaticText(paths_panel, label=self.get_txt("settings_temp_dir")), 0, wx.BOTTOM, 5)
path_box = wx.StaticBox(panel, label=self.get_txt("settings_path_group")) self.temp_dir_picker = wx.DirPickerCtrl(paths_panel, path=self.settings.get("temp_dir", "C:\\Temp_SP"), style=wx.DIRP_DIR_MUST_EXIST)
path_sizer = wx.StaticBoxSizer(path_box, wx.VERTICAL) paths_inner.Add(self.temp_dir_picker, 0, wx.EXPAND | wx.BOTTOM, 15)
path_sizer.Add(wx.StaticText(panel, label=self.get_txt("settings_temp_dir")), 0, wx.BOTTOM, 5) paths_inner.Add(wx.StaticText(paths_panel, label=self.get_txt("settings_app_path")), 0, wx.BOTTOM, 5)
self.temp_dir_picker = wx.DirPickerCtrl(panel, path=self.settings.get("temp_dir", "C:\\Temp_SP"), app_path_box = wx.TextCtrl(paths_panel, value=CONFIG_DIR, style=wx.TE_READONLY | wx.BORDER_NONE)
style=wx.DIRP_DIR_MUST_EXIST) app_path_box.SetBackgroundColour(paths_panel.GetBackgroundColour())
path_sizer.Add(self.temp_dir_picker, 0, wx.EXPAND | wx.BOTTOM, 10) paths_inner.Add(app_path_box, 0, wx.EXPAND | wx.BOTTOM, 15)
path_sizer.Add(wx.StaticText(panel, label=self.get_txt("settings_app_path")), 0, wx.BOTTOM, 5) paths_inner.Add(wx.StaticText(paths_panel, label=self.get_txt("settings_active_temp_path")), 0, wx.BOTTOM, 5)
app_path_box = wx.TextCtrl(panel, value=CONFIG_DIR, style=wx.TE_READONLY | wx.BORDER_NONE) temp_path_box = wx.TextCtrl(paths_panel, value=TEMP_DIR, style=wx.TE_READONLY | wx.BORDER_NONE)
app_path_box.SetBackgroundColour(panel.GetBackgroundColour()) temp_path_box.SetBackgroundColour(paths_panel.GetBackgroundColour())
path_sizer.Add(app_path_box, 0, wx.EXPAND | wx.BOTTOM, 10) paths_inner.Add(temp_path_box, 0, wx.EXPAND)
path_sizer.Add(wx.StaticText(panel, label=self.get_txt("settings_active_temp_path")), 0, wx.BOTTOM, 5) paths_vbox.Add(paths_inner, 1, wx.EXPAND | wx.ALL, 15)
temp_path_box = wx.TextCtrl(panel, value=TEMP_DIR, style=wx.TE_READONLY | wx.BORDER_NONE) paths_panel.SetSizer(paths_vbox)
temp_path_box.SetBackgroundColour(panel.GetBackgroundColour()) self.nb.AddPage(paths_panel, self.get_txt("settings_path_group"))
path_sizer.Add(temp_path_box, 0, wx.EXPAND)
inner_vbox.Add(path_sizer, 0, wx.EXPAND | wx.ALL, 10) # 3. LICENSE TAB
lic_panel = wx.Panel(self.nb)
lic_vbox = wx.BoxSizer(wx.VERTICAL)
lic_inner = wx.BoxSizer(wx.VERTICAL)
# --- Group: License --- lic_inner.Add(wx.StaticText(lic_panel, label=self.get_txt("settings_license_key")), 0, wx.BOTTOM, 5)
lic_box = wx.StaticBox(panel, label=self.get_txt("settings_license_group")) self.license_ctrl = wx.TextCtrl(lic_panel, value=self.settings.get("license_key", ""))
lic_sizer = wx.StaticBoxSizer(lic_box, wx.VERTICAL) lic_inner.Add(self.license_ctrl, 0, wx.EXPAND | wx.BOTTOM, 5)
lic_sizer.Add(wx.StaticText(panel, label=self.get_txt("settings_license_key")), 0, wx.BOTTOM, 5) status_txt = wx.StaticText(lic_panel, label=self.get_txt("settings_license_status"))
self.license_ctrl = wx.TextCtrl(panel, value=self.settings.get("license_key", ""))
lic_sizer.Add(self.license_ctrl, 0, wx.EXPAND | wx.BOTTOM, 5)
status_txt = wx.StaticText(panel, label=self.get_txt("settings_license_status"))
status_txt.SetForegroundColour(wx.RED) status_txt.SetForegroundColour(wx.RED)
lic_sizer.Add(status_txt, 0, wx.TOP, 5) lic_inner.Add(status_txt, 0, wx.TOP, 5)
inner_vbox.Add(lic_sizer, 0, wx.EXPAND | wx.ALL, 10) lic_vbox.Add(lic_inner, 1, wx.EXPAND | wx.ALL, 15)
lic_panel.SetSizer(lic_vbox)
self.nb.AddPage(lic_panel, self.get_txt("settings_license_group").split("/")[0].strip())
# --- Group: Language --- # 4. SYSTEM TAB
lang_box = wx.StaticBox(panel, label=self.get_txt("settings_lang_group")) sys_panel = wx.Panel(self.nb)
lang_sizer = wx.StaticBoxSizer(lang_box, wx.HORIZONTAL) sys_vbox = wx.BoxSizer(wx.VERTICAL)
sys_inner = wx.BoxSizer(wx.VERTICAL)
lang_sizer.Add(wx.StaticText(panel, label=self.get_txt("settings_language")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 10) lang_hbox = wx.BoxSizer(wx.HORIZONTAL)
self.lang_choice = wx.Choice(panel, choices=["Dansk", "English"]) lang_hbox.Add(wx.StaticText(sys_panel, label=self.get_txt("settings_language")), 0, wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, 10)
self.lang_choice = wx.Choice(sys_panel, choices=["Dansk", "English"])
self.lang_choice.SetSelection(0 if self.settings.get("language") == "da" else 1) self.lang_choice.SetSelection(0 if self.settings.get("language") == "da" else 1)
lang_sizer.Add(self.lang_choice, 1, wx.EXPAND) lang_hbox.Add(self.lang_choice, 1, wx.EXPAND)
sys_inner.Add(lang_hbox, 0, wx.EXPAND | wx.BOTTOM, 15)
inner_vbox.Add(lang_sizer, 0, wx.EXPAND | wx.ALL, 10) self.logging_cb = wx.CheckBox(sys_panel, label=self.get_txt("settings_logging"))
# --- Group: Miscellaneous ---
misc_box = wx.StaticBox(panel, label=self.get_txt("settings_logging_group"))
misc_sizer = wx.StaticBoxSizer(misc_box, wx.VERTICAL)
self.logging_cb = wx.CheckBox(panel, label=self.get_txt("settings_logging"))
self.logging_cb.SetValue(self.settings.get("logging_enabled", True)) self.logging_cb.SetValue(self.settings.get("logging_enabled", True))
misc_sizer.Add(self.logging_cb, 0, wx.ALL, 10) sys_inner.Add(self.logging_cb, 0, wx.ALL, 5)
inner_vbox.Add(misc_sizer, 0, wx.EXPAND | wx.ALL, 10) sys_vbox.Add(sys_inner, 1, wx.EXPAND | wx.ALL, 15)
sys_panel.SetSizer(sys_vbox)
self.nb.AddPage(sys_panel, self.get_txt("settings_logging_group").split("/")[0].strip())
panel.SetSizer(inner_vbox) # 5. ABOUT TAB
inner_vbox.Fit(panel) about_panel = wx.Panel(self.nb)
vbox.Add(panel, 1, wx.EXPAND | wx.ALL, 0) about_vbox = wx.BoxSizer(wx.VERTICAL)
# --- Buttons --- about_info = wx.TextCtrl(about_panel, value=self.get_txt("settings_credits"), style=wx.TE_MULTILINE | wx.TE_READONLY | wx.BORDER_NONE)
about_info.SetBackgroundColour(about_panel.GetBackgroundColour())
about_vbox.Add(about_info, 1, wx.EXPAND | wx.ALL, 15)
about_panel.SetSizer(about_vbox)
self.nb.AddPage(about_panel, self.get_txt("settings_about_group"))
vbox.Add(self.nb, 1, wx.EXPAND | wx.ALL, 10)
# --- BUTTONS ---
btn_hbox = wx.BoxSizer(wx.HORIZONTAL) btn_hbox = wx.BoxSizer(wx.HORIZONTAL)
save_btn = wx.Button(self, label=self.get_txt("settings_save"), size=(150, 35)) save_btn = wx.Button(self, label=self.get_txt("settings_save"), size=(150, 35))
save_btn.SetBackgroundColour(wx.Colour(0, 120, 215)) # SharePoint Blue save_btn.SetBackgroundColour(wx.Colour(0, 120, 215)) # SharePoint Blue
@@ -652,9 +694,11 @@ class SharePointApp(wx.Frame):
self.current_items = [] # Gemmer graf-objekterne for rækkerne self.current_items = [] # Gemmer graf-objekterne for rækkerne
self.tree_item_data = {} # Mappenoder -> {type, id, name, drive_id, path} self.tree_item_data = {} # Mappenoder -> {type, id, name, drive_id, path}
self.current_path_data = [] # Gemmer data-objekterne for den nuværende sti (brødkrummer) self.current_path_data = [] # Gemmer data-objekterne for den nuværende sti (brødkrummer)
self._nav_gen = 0 # Incremented on each navigation to discard stale fetch results
self.tree_root = None self.tree_root = None
self.is_navigating_back = False self.is_navigating_back = False
self.active_edits = {} # item_id -> { "name": name, "event": Event, "waiting": bool } self.active_edits = {} # item_id -> { "name": name, "event": Event, "waiting": bool }
self._edits_lock = threading.Lock()
self.favorites = settings.get("favorites", []) self.favorites = settings.get("favorites", [])
self.fav_visible = settings.get("fav_visible", True) self.fav_visible = settings.get("fav_visible", True)
self.sort_col = 0 # Default (Navn) self.sort_col = 0 # Default (Navn)
@@ -688,7 +732,7 @@ class SharePointApp(wx.Frame):
try: try:
self.msal_app = msal.PublicClientApplication(CLIENT_ID, authority=AUTHORITY) self.msal_app = msal.PublicClientApplication(CLIENT_ID, authority=AUTHORITY)
except Exception as e: except Exception as e:
print(f"MSAL Init Error: {e}") logger.error(f"MSAL Init Error: {e}")
self.InitUI() self.InitUI()
self.Centre() self.Centre()
@@ -717,7 +761,7 @@ class SharePointApp(wx.Frame):
if kwargs: if kwargs:
try: try:
return text.format(**kwargs) return text.format(**kwargs)
except: except Exception:
pass pass
return text return text
@@ -911,7 +955,6 @@ class SharePointApp(wx.Frame):
# Add a Gauge to the status bar # Add a Gauge to the status bar
self.gauge = wx.Gauge(self.status_bar, range=100, size=(140, 18), style=wx.GA_HORIZONTAL | wx.GA_SMOOTH) self.gauge = wx.Gauge(self.status_bar, range=100, size=(140, 18), style=wx.GA_HORIZONTAL | wx.GA_SMOOTH)
self.gauge.Hide() self.gauge.Hide()
self.Bind(wx.EVT_SIZE, self.on_status_bar_resize)
panel.SetSizer(vbox) panel.SetSizer(vbox)
self.Bind(wx.EVT_SIZE, self.on_resize) self.Bind(wx.EVT_SIZE, self.on_resize)
@@ -1151,7 +1194,7 @@ class SharePointApp(wx.Frame):
status_text = "Sletter" if self.lang == "da" else "Deleting" status_text = "Sletter" if self.lang == "da" else "Deleting"
self.set_status(f"{status_text} {count+1}/{total}: '{item['name']}'...") self.set_status(f"{status_text} {count+1}/{total}: '{item['name']}'...")
url = f"https://graph.microsoft.com/v1.0/drives/{item['drive_id']}/items/{item['id']}" url = f"https://graph.microsoft.com/v1.0/drives/{item['drive_id']}/items/{item['id']}"
res = requests.delete(url, headers=self.headers) res = _graph_request("DELETE", url, headers=self.headers, timeout=30)
if res.status_code in [204, 200]: if res.status_code in [204, 200]:
count += 1 count += 1
else: else:
@@ -1178,7 +1221,7 @@ class SharePointApp(wx.Frame):
url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}:/{filename}:/content" url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}:/{filename}:/content"
try: try:
with open(local_path, 'rb') as f: with open(local_path, 'rb') as f:
res = requests.put(url, headers=self.headers, data=f) res = requests.put(url, headers=self.headers, data=f, timeout=120)
if res.status_code in [200, 201]: if res.status_code in [200, 201]:
self.set_status(self.get_txt("msg_upload_success", name=filename)) self.set_status(self.get_txt("msg_upload_success", name=filename))
self._refresh_current_view() self._refresh_current_view()
@@ -1219,7 +1262,7 @@ class SharePointApp(wx.Frame):
def _create_folder_sync(self, name, drive_id, parent_id): def _create_folder_sync(self, name, drive_id, parent_id):
url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}/children" url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}/children"
body = {"name": name, "folder": {}, "@microsoft.graph.conflictBehavior": "rename"} body = {"name": name, "folder": {}, "@microsoft.graph.conflictBehavior": "rename"}
res = requests.post(url, headers=self.headers, json=body) res = _graph_request("POST", url, headers=self.headers, json=body, timeout=30)
if res.status_code in [200, 201]: if res.status_code in [200, 201]:
return res.json().get('id') return res.json().get('id')
return None return None
@@ -1229,7 +1272,7 @@ class SharePointApp(wx.Frame):
filename = os.path.basename(local_path) filename = os.path.basename(local_path)
url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}:/{filename}:/content" url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{parent_id}:/{filename}:/content"
with open(local_path, 'rb') as f: with open(local_path, 'rb') as f:
requests.put(url, headers=self.headers, data=f) requests.put(url, headers=self.headers, data=f, timeout=120)
def on_new_folder_clicked(self, event): def on_new_folder_clicked(self, event):
if not self.current_drive_id: return if not self.current_drive_id: return
@@ -1263,7 +1306,7 @@ class SharePointApp(wx.Frame):
self.set_status(f"{self.get_txt('msg_rename')}...") self.set_status(f"{self.get_txt('msg_rename')}...")
url = f"https://graph.microsoft.com/v1.0/drives/{item['drive_id']}/items/{item['id']}" url = f"https://graph.microsoft.com/v1.0/drives/{item['drive_id']}/items/{item['id']}"
body = {"name": new_name} body = {"name": new_name}
res = requests.patch(url, headers=self.headers, json=body) res = _graph_request("PATCH", url, headers=self.headers, json=body, timeout=30)
if res.status_code in [200, 201]: if res.status_code in [200, 201]:
self.set_status(self.get_txt("msg_success")) self.set_status(self.get_txt("msg_success"))
self._refresh_current_view() self._refresh_current_view()
@@ -1284,7 +1327,7 @@ class SharePointApp(wx.Frame):
with wx.DirDialog(self, self.get_txt("msg_select_folder"), style=wx.DD_DEFAULT_STYLE | wx.DD_DIR_MUST_EXIST) as dd: with wx.DirDialog(self, self.get_txt("msg_select_folder"), style=wx.DD_DEFAULT_STYLE | wx.DD_DIR_MUST_EXIST) as dd:
if dd.ShowModal() == wx.ID_OK: if dd.ShowModal() == wx.ID_OK:
parent_path = dd.GetPath() parent_path = dd.GetPath()
dest_path = os.path.join(parent_path, item['name']) dest_path = os.path.join(parent_path, os.path.basename(item['name']))
threading.Thread(target=self._download_folder_bg_task, args=(item, dest_path), daemon=True).start() threading.Thread(target=self._download_folder_bg_task, args=(item, dest_path), daemon=True).start()
def _download_file_bg_task(self, item, dest_path): def _download_file_bg_task(self, item, dest_path):
@@ -1300,7 +1343,7 @@ class SharePointApp(wx.Frame):
def _download_file_sync_call(self, drive_id, item_id, dest_path): def _download_file_sync_call(self, drive_id, item_id, dest_path):
url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{item_id}/content" url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{item_id}/content"
res = requests.get(url, headers=self.headers) res = _graph_request("GET", url, headers=self.headers, timeout=30)
if res.status_code == 200: if res.status_code == 200:
with open(dest_path, 'wb') as f: with open(dest_path, 'wb') as f:
f.write(res.content) f.write(res.content)
@@ -1319,12 +1362,12 @@ class SharePointApp(wx.Frame):
url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{folder_id}/children" url = f"https://graph.microsoft.com/v1.0/drives/{drive_id}/items/{folder_id}/children"
while url: while url:
res = requests.get(url, headers=self.headers) res = _graph_request("GET", url, headers=self.headers, timeout=30)
if res.status_code == 200: if res.status_code == 200:
res_data = res.json() res_data = res.json()
items = res_data.get('value', []) items = res_data.get('value', [])
for item in items: for item in items:
item_path = os.path.join(local_dir, item['name']) item_path = os.path.join(local_dir, os.path.basename(item['name']))
if 'folder' in item: if 'folder' in item:
self._download_folder_recursive_sync(drive_id, item['id'], item_path) self._download_folder_recursive_sync(drive_id, item['id'], item_path)
else: else:
@@ -1342,6 +1385,9 @@ class SharePointApp(wx.Frame):
if start: if start:
self.gauge.Show() self.gauge.Show()
self.gauge.Pulse() self.gauge.Pulse()
rect = self.status_bar.GetFieldRect(1)
self.gauge.SetPosition((rect.x + 5, rect.y + 2))
self.gauge.SetSize((rect.width - 10, rect.height - 4))
else: else:
self.gauge.Hide() self.gauge.Hide()
self.gauge.SetValue(0) self.gauge.SetValue(0)
@@ -1360,7 +1406,8 @@ class SharePointApp(wx.Frame):
wx.CallAfter(_do) wx.CallAfter(_do)
def on_done_editing_clicked(self, event): def on_done_editing_clicked(self, event):
waiting_files = [fid for fid, d in self.active_edits.items() if d.get("waiting")] with self._edits_lock:
waiting_files = [fid for fid, d in self.active_edits.items() if d.get("waiting")]
if not waiting_files: if not waiting_files:
return return
@@ -1531,13 +1578,11 @@ class SharePointApp(wx.Frame):
self.compact_mode = False self.compact_mode = False
self._update_button_labels(full=True) self._update_button_labels(full=True)
event.Skip()
def on_status_bar_resize(self, event):
if hasattr(self, 'gauge') and self.gauge: if hasattr(self, 'gauge') and self.gauge:
rect = self.status_bar.GetFieldRect(1) rect = self.status_bar.GetFieldRect(1)
self.gauge.SetPosition((rect.x + 5, rect.y + 2)) self.gauge.SetPosition((rect.x + 5, rect.y + 2))
self.gauge.SetSize((rect.width - 10, rect.height - 4)) self.gauge.SetSize((rect.width - 10, rect.height - 4))
event.Skip() event.Skip()
def _update_button_labels(self, full=True): def _update_button_labels(self, full=True):
@@ -1624,7 +1669,7 @@ class SharePointApp(wx.Frame):
btn.Bind(wx.EVT_BUTTON, self.load_sites) btn.Bind(wx.EVT_BUTTON, self.load_sites)
elif data: elif data:
def on_click(e, d=data): def on_click(e, d=data):
self._navigate_to_item_data(d, is_breadcrumb=True) self._navigate_to_item_data(d)
# Efter navigation, prøv at finde og vælge den i træet # Efter navigation, prøv at finde og vælge den i træet
wx.CallAfter(self._sync_tree_selection_by_path, d["path"]) wx.CallAfter(self._sync_tree_selection_by_path, d["path"])
@@ -1646,7 +1691,7 @@ class SharePointApp(wx.Frame):
self.headers = {'Authorization': f'Bearer {self.access_token}'} self.headers = {'Authorization': f'Bearer {self.access_token}'}
return True return True
except Exception as e: except Exception as e:
print(f"Token refresh error: {e}") logger.error(f"Token refresh error: {e}")
self.set_status(self.get_txt("status_login_needed")) self.set_status(self.get_txt("status_login_needed"))
return False return False
@@ -1697,13 +1742,12 @@ class SharePointApp(wx.Frame):
url = "https://graph.microsoft.com/v1.0/sites?search=*" url = "https://graph.microsoft.com/v1.0/sites?search=*"
while url: while url:
res = requests.get(url, headers=self.headers) res = _graph_request("GET", url, headers=self.headers, timeout=30)
if res.status_code == 200: if res.status_code == 200:
data = res.json() data = res.json()
all_sites.extend(data.get('value', [])) all_sites.extend(data.get('value', []))
url = data.get('@odata.nextLink') url = data.get('@odata.nextLink')
self.set_status(f"{self.get_txt('status_fetching_sites')} ({len(all_sites)}...)") self.set_status(f"{self.get_txt('status_fetching_sites')} ({len(all_sites)}...)")
self.pulse_gauge(True)
else: else:
break break
@@ -1754,6 +1798,7 @@ class SharePointApp(wx.Frame):
if not self.ensure_valid_token(): return if not self.ensure_valid_token(): return
self.pulse_gauge(True) self.pulse_gauge(True)
all_children = [] all_children = []
url = None
if data['type'] == "SITE": if data['type'] == "SITE":
url = f"https://graph.microsoft.com/v1.0/sites/{data['id']}/drives" url = f"https://graph.microsoft.com/v1.0/sites/{data['id']}/drives"
@@ -1763,12 +1808,11 @@ class SharePointApp(wx.Frame):
url = f"https://graph.microsoft.com/v1.0/drives/{data['drive_id']}/items/{data['id']}/children" url = f"https://graph.microsoft.com/v1.0/drives/{data['drive_id']}/items/{data['id']}/children"
while url: while url:
res = requests.get(url, headers=self.headers) res = _graph_request("GET", url, headers=self.headers, timeout=30)
if res.status_code == 200: if res.status_code == 200:
res_data = res.json() res_data = res.json()
all_children.extend(res_data.get('value', [])) all_children.extend(res_data.get('value', []))
url = res_data.get('@odata.nextLink') url = res_data.get('@odata.nextLink')
self.pulse_gauge(True)
else: else:
break break
@@ -1834,10 +1878,10 @@ class SharePointApp(wx.Frame):
self._navigate_to_item_data(data, tree_item=item) self._navigate_to_item_data(data, tree_item=item)
def _navigate_to_item_data(self, data, tree_item=None, is_breadcrumb=False): def _navigate_to_item_data(self, data, tree_item=None):
try: try:
# Race-condition beskyttelse: Hvis vi allerede er der, så stop (undtagen ved brødkrumme-klik) # Race-condition beskyttelse: Hvis vi allerede er der, så stop
if not is_breadcrumb and getattr(self, 'current_path', None) == data.get("path"): if getattr(self, 'current_path', None) == data.get("path"):
return return
self.current_path = data["path"] self.current_path = data["path"]
@@ -1867,11 +1911,13 @@ class SharePointApp(wx.Frame):
self.current_items = [] self.current_items = []
self.set_status(self.get_txt("status_loading_content")) self.set_status(self.get_txt("status_loading_content"))
threading.Thread(target=self._fetch_list_contents_bg, args=(data,), daemon=True).start() self._nav_gen += 1
gen = self._nav_gen
threading.Thread(target=self._fetch_list_contents_bg, args=(data, gen), daemon=True).start()
except RuntimeError: except RuntimeError:
pass pass
def _fetch_list_contents_bg(self, data): def _fetch_list_contents_bg(self, data, nav_gen=None):
if not self.ensure_valid_token(): return if not self.ensure_valid_token(): return
self.pulse_gauge(True) self.pulse_gauge(True)
items_data = [] items_data = []
@@ -1885,7 +1931,7 @@ class SharePointApp(wx.Frame):
first_chunk = True first_chunk = True
while url: while url:
res = requests.get(url, headers=self.headers) res = _graph_request("GET", url, headers=self.headers, timeout=30)
if res.status_code != 200: break if res.status_code != 200: break
res_data = res.json() res_data = res.json()
@@ -1917,8 +1963,7 @@ class SharePointApp(wx.Frame):
}) })
items_data.extend(chunk_data) items_data.extend(chunk_data)
self.set_status(f"Henter... ({len(items_data)} emner)") self.set_status(self.get_txt("status_loading_items").format(count=len(items_data)))
self.pulse_gauge(True)
# Chunked UI Update # Chunked UI Update
if first_chunk: if first_chunk:
@@ -1930,7 +1975,7 @@ class SharePointApp(wx.Frame):
url = res_data.get('@odata.nextLink') url = res_data.get('@odata.nextLink')
# Finalize # Finalize
wx.CallAfter(self._finalize_list_loading, items_data) wx.CallAfter(self._finalize_list_loading, items_data, nav_gen)
self.pulse_gauge(False) self.pulse_gauge(False)
def _append_list_items(self, items): def _append_list_items(self, items):
@@ -1943,7 +1988,6 @@ class SharePointApp(wx.Frame):
img_idx = self.idx_file img_idx = self.idx_file
if item['type'] == "FOLDER": img_idx = self.idx_folder if item['type'] == "FOLDER": img_idx = self.idx_folder
elif item['type'] == "DRIVE": img_idx = self.idx_drive elif item['type'] == "DRIVE": img_idx = self.idx_drive
elif item['type'] == "SITE": img_idx = self.idx_site
elif item['type'] == "FILE": elif item['type'] == "FILE":
img_idx = self.get_icon_idx_for_file(item['name']) img_idx = self.get_icon_idx_for_file(item['name'])
@@ -1954,8 +1998,10 @@ class SharePointApp(wx.Frame):
self.list_ctrl.SetItem(idx, 2, size_str) self.list_ctrl.SetItem(idx, 2, size_str)
self.list_ctrl.SetItem(idx, 3, item['modified']) self.list_ctrl.SetItem(idx, 3, item['modified'])
def _finalize_list_loading(self, items_data): def _finalize_list_loading(self, items_data, nav_gen=None):
if not self: return if not self: return
if nav_gen is not None and nav_gen != self._nav_gen:
return # User navigated away; discard stale results
self.current_items = items_data self.current_items = items_data
self.apply_sorting() self.apply_sorting()
self.set_status(self.get_txt("status_ready")) self.set_status(self.get_txt("status_ready"))
@@ -2052,7 +2098,7 @@ class SharePointApp(wx.Frame):
idx = self.image_list.Add(bmp) idx = self.image_list.Add(bmp)
self.ext_icons[ext] = idx self.ext_icons[ext] = idx
return idx return idx
except: except Exception:
pass pass
self.ext_icons[ext] = self.idx_file self.ext_icons[ext] = self.idx_file
@@ -2211,11 +2257,15 @@ class SharePointApp(wx.Frame):
file_name = item['name'] file_name = item['name']
drive_id = item['drive_id'] drive_id = item['drive_id']
if item_id in self.active_edits: with self._edits_lock:
already_editing = item_id in self.active_edits
at_limit = len(self.active_edits) >= 10
# UI dialogs are called outside the lock to avoid holding it during blocking calls
if already_editing:
self.show_info(f"'{file_name}' er allerede ved at blive redigeret.", wx.ICON_INFORMATION) self.show_info(f"'{file_name}' er allerede ved at blive redigeret.", wx.ICON_INFORMATION)
return return
if at_limit:
if len(self.active_edits) >= 10:
wx.MessageBox("Du kan kun have 10 filer åbne til redigering ad gangen.", "Maksimum grænse nået", wx.OK | wx.ICON_WARNING) wx.MessageBox("Du kan kun have 10 filer åbne til redigering ad gangen.", "Maksimum grænse nået", wx.OK | wx.ICON_WARNING)
return return
@@ -2252,7 +2302,8 @@ class SharePointApp(wx.Frame):
if not self.ensure_valid_token(): return if not self.ensure_valid_token(): return
edit_event = threading.Event() edit_event = threading.Event()
self.active_edits[item_id] = {"name": file_name, "event": edit_event, "waiting": False} with self._edits_lock:
self.active_edits[item_id] = {"name": file_name, "event": edit_event, "waiting": False}
self.update_edit_ui() self.update_edit_ui()
try: try:
@@ -2269,7 +2320,7 @@ class SharePointApp(wx.Frame):
# 2. Download # 2. Download
self.set_status(self.get_txt("msg_fetching_file", name=file_name)) self.set_status(self.get_txt("msg_fetching_file", name=file_name))
res = requests.get(f"{base_url}/content", headers=self.headers) res = _graph_request("GET", f"{base_url}/content", headers=self.headers, timeout=30)
if res.status_code != 200: if res.status_code != 200:
raise Exception(f"{self.get_txt('msg_unknown_error')}: {res.status_code}") raise Exception(f"{self.get_txt('msg_unknown_error')}: {res.status_code}")
@@ -2306,7 +2357,7 @@ class SharePointApp(wx.Frame):
# Checkout # Checkout
is_checked_out = False is_checked_out = False
checkout_res = requests.post(f"{base_url}/checkout", headers=self.headers) checkout_res = _graph_request("POST", f"{base_url}/checkout", headers=self.headers, timeout=30)
if checkout_res.status_code in [200, 201, 204]: if checkout_res.status_code in [200, 201, 204]:
is_checked_out = True is_checked_out = True
logger.info(f"Fil {file_name} udtjekket succesfuldt.") logger.info(f"Fil {file_name} udtjekket succesfuldt.")
@@ -2339,13 +2390,15 @@ class SharePointApp(wx.Frame):
else: else:
self.set_status(self.get_txt("msg_waiting_for_file", name=file_name)) self.set_status(self.get_txt("msg_waiting_for_file", name=file_name))
edit_event.clear() edit_event.clear()
self.active_edits[item_id]["waiting"] = True with self._edits_lock:
self.active_edits[item_id]["waiting"] = True
self.update_edit_ui() self.update_edit_ui()
edit_event.wait() edit_event.wait()
if item_id in self.active_edits: with self._edits_lock:
self.active_edits[item_id]["waiting"] = False if item_id in self.active_edits:
self.active_edits[item_id]["waiting"] = False
self.update_edit_ui() self.update_edit_ui()
# 4. Tjek om noget er ændret # 4. Tjek om noget er ændret
@@ -2366,7 +2419,7 @@ class SharePointApp(wx.Frame):
if is_checked_out: if is_checked_out:
logger.info(f"Annullerer udtjekning (discardCheckout) for {file_name}...") logger.info(f"Annullerer udtjekning (discardCheckout) for {file_name}...")
res = requests.post(f"{base_url}/discardCheckout", headers=self.headers) res = _graph_request("POST", f"{base_url}/discardCheckout", headers=self.headers, timeout=30)
if res.status_code in [200, 204]: if res.status_code in [200, 204]:
is_checked_out = False is_checked_out = False
else: else:
@@ -2374,14 +2427,14 @@ class SharePointApp(wx.Frame):
logger.info(f"Ændring fundet! Uploader {file_name}...") logger.info(f"Ændring fundet! Uploader {file_name}...")
self.set_status(self.get_txt("msg_updating_changes")) self.set_status(self.get_txt("msg_updating_changes"))
with open(local_path, 'rb') as f: with open(local_path, 'rb') as f:
upload_res = requests.put(f"{base_url}/content", headers=self.headers, data=f) upload_res = requests.put(f"{base_url}/content", headers=self.headers, data=f, timeout=120)
if upload_res.status_code not in [200, 201]: if upload_res.status_code not in [200, 201]:
raise Exception(f"{self.get_txt('msg_update_failed_code', code=upload_res.status_code)}") raise Exception(f"{self.get_txt('msg_update_failed_code', code=upload_res.status_code)}")
# 6. Checkin (Kun hvis vi faktisk uploadede noget) # 6. Checkin (Kun hvis vi faktisk uploadede noget)
if is_checked_out: if is_checked_out:
self.set_status(self.get_txt("msg_checking_in", name=file_name)) self.set_status(self.get_txt("msg_checking_in", name=file_name))
res = requests.post(f"{base_url}/checkin", headers=self.headers, json={"comment": "SP Explorer Edit"}) res = _graph_request("POST", f"{base_url}/checkin", headers=self.headers, json={"comment": "SP Explorer Edit"}, timeout=30)
if res.status_code in [200, 201, 204]: if res.status_code in [200, 201, 204]:
is_checked_out = False is_checked_out = False
@@ -2389,7 +2442,7 @@ class SharePointApp(wx.Frame):
try: try:
os.remove(local_path) os.remove(local_path)
os.rmdir(working_dir) os.rmdir(working_dir)
except: except Exception:
pass pass
self.set_status(self.get_txt("msg_update_success", name=file_name)) self.set_status(self.get_txt("msg_update_success", name=file_name))
@@ -2403,10 +2456,11 @@ class SharePointApp(wx.Frame):
if is_checked_out: if is_checked_out:
# Emergency cleanup hvis vi stadig har fat i filen (f.eks. ved crash eller afbrydelse) # Emergency cleanup hvis vi stadig har fat i filen (f.eks. ved crash eller afbrydelse)
logger.info(f"Rydder op: Kalder discardCheckout for {file_name}...") logger.info(f"Rydder op: Kalder discardCheckout for {file_name}...")
requests.post(f"{base_url}/discardCheckout", headers=self.headers) _graph_request("POST", f"{base_url}/discardCheckout", headers=self.headers, timeout=30)
if item_id in self.active_edits: with self._edits_lock:
del self.active_edits[item_id] if item_id in self.active_edits:
del self.active_edits[item_id]
self.update_edit_ui() self.update_edit_ui()
if __name__ == "__main__": if __name__ == "__main__":

501
tests/test_review_fixes.py Normal file
View File

@@ -0,0 +1,501 @@
"""
Unit tests for the code-review bug fixes applied to sharepoint_browser.py.
Covers:
C1 - url=None prevents UnboundLocalError in _fetch_tree_children_bg
I1 - nav_gen guard in _finalize_list_loading discards stale results
I2 - System tab label derivation from STRINGS key
I3 - status_loading_items translation key present and formattable
C-1 - Refresh calls (nav_gen=None default) always apply their results
S-2 - nav_gen=None sentinel is safer than 0
S-1 - is_breadcrumb parameter removed from _navigate_to_item_data
S2 - Dead SITE branch removed from _append_list_items
All tests run without a live display; wx is imported but no widgets are
instantiated.
"""
import sys
import os
import inspect
import unittest
from unittest.mock import MagicMock, patch
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
import sharepoint_browser as sb
# ---------------------------------------------------------------------------
# I2 + I3: STRINGS dictionary
# ---------------------------------------------------------------------------
class TestStrings(unittest.TestCase):
def test_system_tab_label_da(self):
"""I2: Danish settings_logging_group yields 'System' after split/strip."""
group = sb.STRINGS["da"]["settings_logging_group"]
self.assertEqual(group.split("/")[0].strip(), "System")
def test_system_tab_label_en(self):
"""I2: English settings_logging_group yields 'System' after split/strip."""
group = sb.STRINGS["en"]["settings_logging_group"]
self.assertEqual(group.split("/")[0].strip(), "System")
def test_status_loading_items_present_da(self):
"""I3: status_loading_items key exists in Danish STRINGS."""
self.assertIn("status_loading_items", sb.STRINGS["da"])
def test_status_loading_items_present_en(self):
"""I3: status_loading_items key exists in English STRINGS."""
self.assertIn("status_loading_items", sb.STRINGS["en"])
def test_status_loading_items_format_da(self):
"""I3: Danish template formats with named {count} argument."""
result = sb.STRINGS["da"]["status_loading_items"].format(count=42)
self.assertIn("42", result)
def test_status_loading_items_format_en(self):
"""I3: English template formats with named {count} argument."""
result = sb.STRINGS["en"]["status_loading_items"].format(count=99)
self.assertIn("99", result)
def test_status_loading_items_da_uses_count_kwarg(self):
"""I3: Danish template uses {count} placeholder (not positional)."""
template = sb.STRINGS["da"]["status_loading_items"]
self.assertIn("{count}", template)
def test_status_loading_items_en_uses_count_kwarg(self):
"""I3: English template uses {count} placeholder (not positional)."""
template = sb.STRINGS["en"]["status_loading_items"]
self.assertIn("{count}", template)
# ---------------------------------------------------------------------------
# I1 + C-1 + S-2: nav_gen guard in _finalize_list_loading
# ---------------------------------------------------------------------------
class TestNavGenGuard(unittest.TestCase):
"""
_finalize_list_loading(self, items_data, nav_gen=None)
Guard logic:
nav_gen is None → always apply (refresh / unconstrained calls)
nav_gen == self._nav_gen → apply (matches current navigation)
nav_gen != self._nav_gen → discard (stale; user navigated away)
"""
def _make_app(self, current_gen: int):
"""Minimal mock that satisfies _finalize_list_loading's needs."""
app = MagicMock()
app._nav_gen = current_gen
# MagicMock is truthy by default so `if not self` passes
return app
# --- nav_gen=None cases (C-1 / S-2) ---
def test_none_gen_applies_when_nav_gen_is_1(self):
"""C-1/S-2: nav_gen=None applies results regardless of _nav_gen."""
app = self._make_app(1)
items = [{"name": "a"}]
sb.SharePointApp._finalize_list_loading(app, items, nav_gen=None)
self.assertEqual(app.current_items, items)
app.apply_sorting.assert_called_once()
def test_none_gen_applies_when_nav_gen_is_high(self):
"""C-1: nav_gen=None still applies when _nav_gen is large."""
app = self._make_app(99)
items = [{"name": "b"}]
sb.SharePointApp._finalize_list_loading(app, items, nav_gen=None)
self.assertEqual(app.current_items, items)
def test_default_gen_arg_applies(self):
"""C-1: Omitting nav_gen entirely (default=None) always applies."""
app = self._make_app(5)
items = [{"name": "refresh"}]
sb.SharePointApp._finalize_list_loading(app, items) # no nav_gen kwarg
self.assertEqual(app.current_items, items)
app.apply_sorting.assert_called_once()
# --- matching gen (I1, happy path) ---
def test_matching_gen_applies(self):
"""I1: Results applied when nav_gen matches _nav_gen."""
app = self._make_app(3)
items = [{"name": "c"}]
sb.SharePointApp._finalize_list_loading(app, items, nav_gen=3)
self.assertEqual(app.current_items, items)
app.apply_sorting.assert_called_once()
# --- stale gen cases (I1) ---
def test_stale_gen_discards_results(self):
"""I1: Results discarded when nav_gen < _nav_gen (user navigated away)."""
app = self._make_app(5)
sentinel = object()
app.current_items = sentinel
sb.SharePointApp._finalize_list_loading(app, [{"name": "old"}], nav_gen=2)
self.assertIs(app.current_items, sentinel,
"current_items was overwritten by stale result")
app.apply_sorting.assert_not_called()
def test_future_gen_discards_results(self):
"""I1: Results discarded when nav_gen > _nav_gen (shouldn't happen, but safe)."""
app = self._make_app(3)
sentinel = object()
app.current_items = sentinel
sb.SharePointApp._finalize_list_loading(app, [{"name": "future"}], nav_gen=99)
self.assertIs(app.current_items, sentinel)
app.apply_sorting.assert_not_called()
def test_gen_zero_still_discarded_when_nav_gen_nonzero(self):
"""S-2: nav_gen=0 (old broken default) is now treated as a stale gen, not a pass."""
app = self._make_app(1)
sentinel = object()
app.current_items = sentinel
sb.SharePointApp._finalize_list_loading(app, [{"name": "zero"}], nav_gen=0)
self.assertIs(app.current_items, sentinel,
"nav_gen=0 should be treated as stale, not as 'no constraint'")
app.apply_sorting.assert_not_called()
# ---------------------------------------------------------------------------
# C1: url=None initialization in _fetch_tree_children_bg
# ---------------------------------------------------------------------------
class TestUrlInitialization(unittest.TestCase):
def test_url_initialized_to_none_before_conditional(self):
"""C1: _fetch_tree_children_bg initializes url=None before the if block."""
source = inspect.getsource(sb.SharePointApp._fetch_tree_children_bg)
# Find the line that sets url = None before the if data['type'] conditional
lines = source.splitlines()
url_none_idx = next(
(i for i, l in enumerate(lines) if "url = None" in l), None
)
site_if_idx = next(
(i for i, l in enumerate(lines)
if 'data[\'type\'] == "SITE"' in l or 'data["type"] == "SITE"' in l),
None
)
self.assertIsNotNone(url_none_idx,
"_fetch_tree_children_bg has no 'url = None' initializer")
self.assertIsNotNone(site_if_idx,
"_fetch_tree_children_bg has no SITE type conditional")
self.assertLess(url_none_idx, site_if_idx,
"'url = None' must appear before the if data['type'] block")
# ---------------------------------------------------------------------------
# S2: Dead SITE branch removed from _append_list_items
# ---------------------------------------------------------------------------
class TestDeadSiteBranch(unittest.TestCase):
def test_append_list_items_has_no_site_img_branch(self):
"""S2: _append_list_items no longer contains the dead SITE image branch."""
source = inspect.getsource(sb.SharePointApp._append_list_items)
self.assertNotIn(
'item[\'type\'] == "SITE"',
source,
"_append_list_items still contains dead SITE branch"
)
# ---------------------------------------------------------------------------
# S-1: is_breadcrumb parameter removed from _navigate_to_item_data
# ---------------------------------------------------------------------------
class TestIsBreakcrumbRemoved(unittest.TestCase):
def test_no_is_breadcrumb_param(self):
"""S-1: _navigate_to_item_data no longer has an is_breadcrumb parameter."""
sig = inspect.signature(sb.SharePointApp._navigate_to_item_data)
self.assertNotIn(
"is_breadcrumb", sig.parameters,
"_navigate_to_item_data still declares is_breadcrumb"
)
def test_tree_item_param_still_present(self):
"""Regression: tree_item parameter was not accidentally removed."""
sig = inspect.signature(sb.SharePointApp._navigate_to_item_data)
self.assertIn("tree_item", sig.parameters)
# ---------------------------------------------------------------------------
# Task 7: bare except → except Exception, print → logger, basename sanitization
# ---------------------------------------------------------------------------
class TestTask7BareExcept(unittest.TestCase):
"""Verify that bare except: clauses have been replaced with except Exception:."""
def _get_source_lines(self):
return inspect.getsource(sb).splitlines()
def test_no_bare_except_in_load_settings(self):
"""7a: load_settings() uses except Exception, not bare except."""
source = inspect.getsource(sb.load_settings)
self.assertNotIn("except:", source,
"load_settings still has a bare except:")
def test_no_bare_except_in_get_txt(self):
"""7a: get_txt() uses except Exception, not bare except."""
source = inspect.getsource(sb.SharePointApp.get_txt)
self.assertNotIn("except:", source,
"get_txt still has a bare except:")
def test_no_bare_except_in_get_icon_idx(self):
"""7a: get_icon_idx_for_file() uses except Exception, not bare except."""
source = inspect.getsource(sb.SharePointApp.get_icon_idx_for_file)
self.assertNotIn("except:", source,
"get_icon_idx_for_file still has a bare except:")
def test_no_bare_except_in_process_file(self):
"""7a: process_file() cleanup block uses except Exception, not bare except."""
source = inspect.getsource(sb.SharePointApp.process_file)
self.assertNotIn("except:", source,
"process_file still has a bare except: in cleanup block")
def test_no_print_in_msal_init(self):
"""7b: MSAL init error uses logger, not print()."""
source = inspect.getsource(sb.SharePointApp.__init__)
self.assertNotIn('print(f"MSAL Init Error', source,
"__init__ still uses print() for MSAL Init Error")
def test_no_print_in_ensure_valid_token(self):
"""7b: ensure_valid_token() uses logger, not print()."""
source = inspect.getsource(sb.SharePointApp.ensure_valid_token)
self.assertNotIn('print(f"Token refresh error', source,
"ensure_valid_token still uses print() for token error")
def test_basename_in_download_folder_recursive(self):
"""7c: _download_folder_recursive_sync uses os.path.basename on item name."""
source = inspect.getsource(sb.SharePointApp._download_folder_recursive_sync)
self.assertIn("os.path.basename", source,
"_download_folder_recursive_sync does not sanitize item['name'] with basename")
def test_basename_in_on_download_clicked(self):
"""7c: on_download_clicked uses os.path.basename when building dest_path for folders."""
source = inspect.getsource(sb.SharePointApp.on_download_clicked)
self.assertIn("os.path.basename", source,
"on_download_clicked does not sanitize item['name'] with basename for folder downloads")
# ---------------------------------------------------------------------------
# Task 8: all requests.* calls must carry a timeout= parameter
# ---------------------------------------------------------------------------
class TestNetworkTimeouts(unittest.TestCase):
"""
Every requests.get/post/put/patch/delete call must include timeout=
so that background threads cannot hang indefinitely on a stalled network.
"""
def _calls_missing_timeout(self):
"""
Return a list of (line_number, line_text) for every requests.*()
call in the source file that is missing a timeout= argument.
Strategy: single-line calls are the norm in this codebase. We look
for lines that contain 'requests.METHOD(' and do NOT also contain
'timeout='. Lines that contain 'timeout=' anywhere on the same line
are considered compliant.
"""
import re
src_path = os.path.join(
os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
"sharepoint_browser.py"
)
# Matches real call sites: assignment or standalone call (not docstrings)
pattern = re.compile(
r'(=\s*|^\s*)requests\.(get|post|put|patch|delete)\('
)
missing = []
with open(src_path, encoding='utf-8') as fh:
for lineno, line in enumerate(fh, 1):
stripped = line.lstrip()
# Skip comment lines and docstring prose (lines that are plain text)
if stripped.startswith('#'):
continue
if pattern.search(line) and 'timeout=' not in line:
missing.append((lineno, line.rstrip()))
return missing
def test_all_requests_calls_have_timeout(self):
"""Task 8: No requests.* call is missing a timeout= parameter."""
missing = self._calls_missing_timeout()
if missing:
details = "\n".join(f" line {n}: {txt}" for n, txt in missing)
self.fail(
f"{len(missing)} requests call(s) are missing timeout=:\n{details}"
)
# ---------------------------------------------------------------------------
# Task 9: _graph_request helper — retry on 429/503 with Retry-After support
# ---------------------------------------------------------------------------
class TestGraphRequest(unittest.TestCase):
"""Unit tests for the _graph_request() module-level helper."""
def test_helper_exists(self):
"""Task 9: _graph_request function is defined at module level."""
self.assertTrue(
callable(getattr(sb, "_graph_request", None)),
"_graph_request is not defined in sharepoint_browser"
)
def test_success_on_first_attempt(self):
"""Task 9: Returns immediately when first response is 200."""
mock_resp = MagicMock()
mock_resp.status_code = 200
with patch("sharepoint_browser.requests.request", return_value=mock_resp) as mock_req:
result = sb._graph_request("GET", "https://example.com/", headers={})
self.assertEqual(result.status_code, 200)
self.assertEqual(mock_req.call_count, 1)
def test_retries_on_429(self):
"""Task 9: Retries when response is 429 (rate limited)."""
responses = [
MagicMock(status_code=429, headers={"Retry-After": "0"}),
MagicMock(status_code=429, headers={"Retry-After": "0"}),
MagicMock(status_code=200, headers={}),
]
with patch("sharepoint_browser.requests.request", side_effect=responses) as mock_req:
with patch("sharepoint_browser.time.sleep"):
result = sb._graph_request("GET", "https://example.com/", headers={})
self.assertEqual(result.status_code, 200)
self.assertEqual(mock_req.call_count, 3)
def test_retries_on_503(self):
"""Task 9: Retries when response is 503 (service unavailable)."""
responses = [
MagicMock(status_code=503, headers={}),
MagicMock(status_code=200, headers={}),
]
with patch("sharepoint_browser.requests.request", side_effect=responses) as mock_req:
with patch("sharepoint_browser.time.sleep"):
result = sb._graph_request("POST", "https://example.com/", headers={})
self.assertEqual(result.status_code, 200)
self.assertEqual(mock_req.call_count, 2)
def test_returns_last_response_after_max_retries(self):
"""Task 9: Returns last 429 response when all retries are exhausted."""
resp_429 = MagicMock(status_code=429, headers={"Retry-After": "0"})
responses = [resp_429] * sb._MAX_RETRIES
with patch("sharepoint_browser.requests.request", side_effect=responses):
with patch("sharepoint_browser.time.sleep"):
result = sb._graph_request("GET", "https://example.com/", headers={})
self.assertEqual(result.status_code, 429)
def test_respects_retry_after_header(self):
"""Task 9: sleep() is called with the Retry-After value from the response."""
responses = [
MagicMock(status_code=429, headers={"Retry-After": "5"}),
MagicMock(status_code=200, headers={}),
]
with patch("sharepoint_browser.requests.request", side_effect=responses):
with patch("sharepoint_browser.time.sleep") as mock_sleep:
sb._graph_request("GET", "https://example.com/", headers={})
mock_sleep.assert_called_once_with(5)
def test_default_timeout_injected(self):
"""Task 9: timeout=30 is injected when caller does not provide one."""
mock_resp = MagicMock(status_code=200, headers={})
with patch("sharepoint_browser.requests.request", return_value=mock_resp) as mock_req:
sb._graph_request("GET", "https://example.com/", headers={})
_, kwargs = mock_req.call_args
self.assertEqual(kwargs.get("timeout"), 30)
def test_caller_timeout_not_overridden(self):
"""Task 9: Explicit timeout from caller is not overwritten by the helper."""
mock_resp = MagicMock(status_code=200, headers={})
with patch("sharepoint_browser.requests.request", return_value=mock_resp) as mock_req:
sb._graph_request("GET", "https://example.com/", headers={}, timeout=60)
_, kwargs = mock_req.call_args
self.assertEqual(kwargs.get("timeout"), 60)
def test_max_retries_constant_exists(self):
"""Task 9: _MAX_RETRIES constant is defined."""
self.assertTrue(
hasattr(sb, "_MAX_RETRIES"),
"_MAX_RETRIES constant not found in sharepoint_browser"
)
def test_no_sleep_after_final_retry(self):
"""Fix #1: sleep() is NOT called after the last exhausted attempt."""
resp_429 = MagicMock(status_code=429, headers={"Retry-After": "0"})
responses = [resp_429] * sb._MAX_RETRIES
with patch("sharepoint_browser.requests.request", side_effect=responses):
with patch("sharepoint_browser.time.sleep") as mock_sleep:
sb._graph_request("GET", "https://example.com/", headers={})
# sleep should be called for the first N-1 failures, NOT the last one
self.assertEqual(
mock_sleep.call_count,
sb._MAX_RETRIES - 1,
f"sleep() called {mock_sleep.call_count} times; expected {sb._MAX_RETRIES - 1} "
f"(no sleep after the final failed attempt)"
)
# ---------------------------------------------------------------------------
# Task 10: threading.Lock for active_edits compound operations
# ---------------------------------------------------------------------------
class TestActiveEditsLock(unittest.TestCase):
"""Verify that _edits_lock exists and guards all compound active_edits operations."""
def test_edits_lock_declared_in_init(self):
"""Task 10: SharePointApp.__init__ creates self._edits_lock."""
source = inspect.getsource(sb.SharePointApp.__init__)
self.assertIn("_edits_lock", source,
"__init__ does not declare _edits_lock")
def test_edits_lock_is_threading_lock(self):
"""Task 10: _edits_lock initialisation uses threading.Lock()."""
source = inspect.getsource(sb.SharePointApp.__init__)
self.assertIn("threading.Lock()", source,
"__init__ does not initialise _edits_lock with threading.Lock()")
def test_open_file_uses_lock(self):
"""Task 10: open_file() acquires _edits_lock before checking active_edits."""
source = inspect.getsource(sb.SharePointApp.open_file)
self.assertIn("_edits_lock", source,
"open_file does not use _edits_lock")
def test_process_file_uses_lock(self):
"""Task 10: process_file() acquires _edits_lock when writing active_edits."""
source = inspect.getsource(sb.SharePointApp.process_file)
self.assertIn("_edits_lock", source,
"process_file does not use _edits_lock")
def test_process_file_lock_on_initial_assign(self):
"""Task 10: active_edits[item_id] = ... assignment is inside a lock block."""
source = inspect.getsource(sb.SharePointApp.process_file)
# Check lock wraps the initial dict assignment
lock_idx = source.find("_edits_lock")
assign_idx = source.find('self.active_edits[item_id] = ')
self.assertGreater(assign_idx, 0,
"active_edits assignment not found in process_file")
# The lock must appear before the assignment
self.assertLess(lock_idx, assign_idx,
"_edits_lock must appear before the active_edits assignment in process_file")
def test_process_file_lock_on_delete(self):
"""Task 10: del active_edits[item_id] is inside a lock block in process_file."""
source = inspect.getsource(sb.SharePointApp.process_file)
self.assertIn("del self.active_edits[item_id]", source,
"del active_edits[item_id] not found in process_file")
# Count lock usages — there should be at least 2
lock_count = source.count("_edits_lock")
self.assertGreaterEqual(lock_count, 2,
f"Expected at least 2 uses of _edits_lock in process_file, found {lock_count}")
def test_on_done_editing_uses_lock(self):
"""Task 10: on_done_editing_clicked acquires lock for active_edits iteration."""
source = inspect.getsource(sb.SharePointApp.on_done_editing_clicked)
self.assertIn("_edits_lock", source,
"on_done_editing_clicked does not use _edits_lock")
if __name__ == "__main__":
unittest.main(verbosity=2)