Compare commits
44 Commits
220bedda48
...
1.1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7fd69a9c3f | ||
|
|
0dfef3e611 | ||
|
|
707645ab36 | ||
|
|
df55660291 | ||
|
|
e8e1d8b60d | ||
|
|
ba3d8dded5 | ||
|
|
d7c0fdd091 | ||
|
|
f168d0c60a | ||
|
|
0cbec6477f | ||
|
|
a55365259c | ||
|
|
d529568798 | ||
|
|
b800f0308d | ||
|
|
dadbb4d51a | ||
|
|
be7a78deb8 | ||
|
|
4abf221887 | ||
|
|
205b1ac241 | ||
|
|
0159b91c69 | ||
|
|
2dfe1a41be | ||
|
|
f11f487ba3 | ||
|
|
ad6055963d | ||
|
|
e14012d2a5 | ||
|
|
62725f9be6 | ||
|
|
1a85f1d963 | ||
|
|
62639632dc | ||
|
|
bff066d3fb | ||
|
|
232ab85cc7 | ||
|
|
044559c913 | ||
|
|
2dd31d19c6 | ||
|
|
6b6387f5c8 | ||
|
|
6ab9901cc8 | ||
|
|
c5f18b520a | ||
|
|
784ca755d5 | ||
|
|
fa6840de75 | ||
|
|
8e53f69e68 | ||
|
|
8f80611d32 | ||
|
|
86ff8043f1 | ||
|
|
0d8407f624 | ||
|
|
6659ebd6ac | ||
|
|
f7cebfc489 | ||
|
|
b1c46fbace | ||
|
|
ed931f2088 | ||
|
|
9ccbcbaf0c | ||
|
|
07913c0224 | ||
|
|
dfbef36558 |
5
.gitignore
vendored
5
.gitignore
vendored
@@ -41,6 +41,11 @@ coverage.xml
|
|||||||
|
|
||||||
# Local settings (keep your secrets safe)
|
# Local settings (keep your secrets safe)
|
||||||
settings.json
|
settings.json
|
||||||
|
searchindex.json
|
||||||
|
|
||||||
# Temporary files
|
# Temporary files
|
||||||
C:\Temp_SP\
|
C:\Temp_SP\
|
||||||
|
|
||||||
|
# Project tooling / AI assistant files
|
||||||
|
tasks/
|
||||||
|
.claude/
|
||||||
|
|||||||
101
README.md
101
README.md
@@ -1,44 +1,89 @@
|
|||||||
# SharePoint Explorer
|
# SharePoint Browser
|
||||||
|
|
||||||
En moderne Python-baseret fil-browser til Microsoft SharePoint, designet til at omgå Windows' `MAX_PATH` (260 karakterer) begrænsning.
|
En moderne Python-baseret fil-browser til Microsoft SharePoint, specielt designet til at omgå Windows' `MAX_PATH` (260 karakterer) begrænsning. Det opnås ved at integrere direkte med Microsoft Graph API ved hjælp af unikke ID'er og downloade/udtjekke filer til redigering via korte, midlertidige stier lokalt.
|
||||||
|
|
||||||
## Funktioner
|
## 🚀 Funktioner
|
||||||
- **Søg og Browse:** Naviger dynamisk gennem SharePoint sites, dokumentbiblioteker og mapper.
|
|
||||||
- **Sikker Redigering:** Automatisk Check-out/Check-in håndtering via Microsoft Graph API.
|
|
||||||
- **Explorer Vibes:** Moderne brugerflade med sortering (mapper øverst) og brødkrummesti (breadcrumb).
|
|
||||||
- **Ingen Sti-begrænsning:** Arbejder med unikke ID'er og korte midlertidige stier for at undgå MAX_PATH fejl.
|
|
||||||
|
|
||||||
## Installation & Udvikling
|
- **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.
|
||||||
|
- **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`).
|
||||||
|
- **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).
|
||||||
|
- **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.
|
||||||
|
|
||||||
|
## ⚙️ 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
|
||||||
|
|
||||||
|
- **Sprog:** Python 3.x
|
||||||
|
- **GUI Framework:** wxPython
|
||||||
|
- **Godkendelse:** MSAL (Microsoft Authentication Library)
|
||||||
|
- **API Integration:** Microsoft Graph API via `requests`
|
||||||
|
- **Fil-overvågning:** Polling via låse på lokalt filsystem
|
||||||
|
|
||||||
|
## 📦 Installation & Opstart
|
||||||
|
|
||||||
### Forudsætninger
|
### Forudsætninger
|
||||||
- Python 3.x
|
Sørg for, at du har Python installeret sammen med afhængighederne. Det anbefales at have en Microsoft 365-licens klar.
|
||||||
- Microsoft 365 licens (Business Standard eller højere anbefales)
|
|
||||||
|
|
||||||
### Setup
|
```bash
|
||||||
1. Installer afhængigheder:
|
pip install wxPython msal requests
|
||||||
```bash
|
```
|
||||||
pip install customtkinter msal requests
|
|
||||||
```
|
|
||||||
2. Konfigurer `settings.json` med din `client_id` og `tenant_id`.
|
|
||||||
|
|
||||||
### Kørsel
|
### Kør applikationen
|
||||||
|
Start op med:
|
||||||
```bash
|
```bash
|
||||||
python sharepoint_browser.py
|
python sharepoint_browser.py
|
||||||
```
|
```
|
||||||
|
|
||||||
## Kompilering til EXE
|
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.
|
||||||
For at pakke programmet til en enkelt selvstændig `.exe` fil:
|
|
||||||
|
## 🏗️ Byg til EXE (Valgfrit)
|
||||||
|
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 --noconsole --onefile --collect-all customtkinter --name "SharePoint Explorer" sharepoint_browser.py
|
python -m PyInstaller "SharePoint Explorer.spec" --noconfirm
|
||||||
```
|
```
|
||||||
Den færdige fil findes i mappen `dist/`. Husk at placere `settings.json` i samme mappe som `.exe` filen.
|
|
||||||
|
|
||||||
## Konfiguration (`settings.json`)
|
Den færdige fil placeres i `dist/SharePoint Explorer.exe` med ikon indlejret.
|
||||||
```json
|
|
||||||
{
|
## 🧩 Arkitektur & Workflow
|
||||||
"client_id": "DIN_CLIENT_ID",
|
1. **Godkendelse:** Autentificerer brugeren via MSAL & MS Graph API.
|
||||||
"tenant_id": "DIN_TENANT_ID",
|
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.
|
||||||
"temp_dir": "C:\\Temp_SP"
|
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.
|
||||||
|
- Hentes ned til det lokale drev under korte stier, eks. `C:\Temp_SP\[MD5-Hash].[ext]`.
|
||||||
|
- Et baggrunds-thread overvåger det lokale program (fx Word) kontinuerligt via `os.rename()` tricket.
|
||||||
|
- 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
|
||||||
|
1. Integration for håndtering af flere tenants (lejemål)
|
||||||
|
2. Et yderligere detaljeret log-system specielt til debugging af baggrundstråden.
|
||||||
|
3. Udvidet aktivitetslog til sporing af handlinger for de seneste 14 dage.
|
||||||
|
4. Styring af licenser til specifikke kunders varigheder (fx 1 år, 3 år, Lifetime).
|
||||||
|
|||||||
4
requirements.txt
Normal file
4
requirements.txt
Normal file
@@ -0,0 +1,4 @@
|
|||||||
|
wxPython>=4.2.1
|
||||||
|
msal>=1.28.0
|
||||||
|
requests>=2.31.0
|
||||||
|
pyinstaller>=6.5.0
|
||||||
File diff suppressed because it is too large
Load Diff
501
tests/test_review_fixes.py
Normal file
501
tests/test_review_fixes.py
Normal 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)
|
||||||
Reference in New Issue
Block a user