diff --git a/src/ccma/storage/repository.py b/src/ccma/storage/repository.py index 15184f4..98ade57 100644 --- a/src/ccma/storage/repository.py +++ b/src/ccma/storage/repository.py @@ -2,9 +2,11 @@ from __future__ import annotations import json import os +import re import shutil import unicodedata from collections.abc import Iterable +from contextlib import contextmanager from datetime import date, datetime, timedelta from decimal import Decimal from pathlib import Path @@ -93,6 +95,7 @@ DEFAULT_CONFIGURATION = { "member_number_policy": { "mode": "automatic", "pattern": DEFAULT_MEMBER_NUMBER_PATTERN, + "allocation_strategy": "next_free", }, "member_number_sequences": {}, "reminder_policy": { @@ -160,6 +163,8 @@ class MemberRepository: policy = config.get("member_number_policy") or {} if str(policy.get("mode", "automatic")) not in {"automatic", "manual"}: errors.append("repository.json: ungültiger Mitgliedsnummernmodus") + if str(policy.get("allocation_strategy", "next_free")) not in {"next_free", "max_plus_one"}: + errors.append("repository.json: ungültige Vergabestrategie für Mitgliedsnummern") validate_member_number_pattern(str(policy.get("pattern", DEFAULT_MEMBER_NUMBER_PATTERN))) except (OSError, ValueError, TypeError, json.JSONDecodeError, RepositoryError) as exc: errors.append(f"repository.json: {exc}") @@ -290,6 +295,28 @@ class MemberRepository: phone: str = "", birth_date: str = "", member_number: str = "", + ) -> Member: + with self._member_number_lock(): + return self._create_member_locked( + first_name=first_name, + last_name=last_name, + nickname=nickname, + email=email, + phone=phone, + birth_date=birth_date, + member_number=member_number, + ) + + def _create_member_locked( + self, + *, + first_name: str, + last_name: str, + nickname: str, + email: str, + phone: str, + birth_date: str, + member_number: str, ) -> Member: if not first_name.strip() or not last_name.strip(): raise RepositoryError("Vorname und Nachname sind erforderlich.") @@ -305,7 +332,9 @@ class MemberRepository: elif policy["mode"] == "manual": raise RepositoryError("Eine Mitgliedsnummer ist erforderlich.") else: - selected_number = self._allocate_member_number(policy["pattern"]) + selected_number = self._allocate_member_number( + policy["pattern"], policy["allocation_strategy"] + ) member_id = str(uuid4()) directory = self._member_path(member_id) directory.mkdir(parents=True, exist_ok=False) @@ -1703,15 +1732,30 @@ class MemberRepository: if mode not in {"automatic", "manual"}: mode = "automatic" pattern = str(policy.get("pattern", DEFAULT_MEMBER_NUMBER_PATTERN)) + allocation_strategy = str(policy.get("allocation_strategy", "next_free")) + if allocation_strategy not in {"next_free", "max_plus_one"}: + allocation_strategy = "next_free" validate_member_number_pattern(pattern) - return {"mode": mode, "pattern": pattern} + return { + "mode": mode, + "pattern": pattern, + "allocation_strategy": allocation_strategy, + } - def save_member_number_policy(self, *, mode: str, pattern: str) -> None: + def save_member_number_policy( + self, *, mode: str, pattern: str, allocation_strategy: str = "next_free" + ) -> None: if mode not in {"automatic", "manual"}: raise RepositoryError("Ungültiger Mitgliedsnummernmodus.") + if allocation_strategy not in {"next_free", "max_plus_one"}: + raise RepositoryError("Ungültige Vergabestrategie für Mitgliedsnummern.") validate_member_number_pattern(pattern) config = read_json(self.root / "repository.json") - config["member_number_policy"] = {"mode": mode, "pattern": pattern.strip()} + config["member_number_policy"] = { + "mode": mode, + "pattern": pattern.strip(), + "allocation_strategy": allocation_strategy, + } config.setdefault("member_number_sequences", {}) write_json_atomic(self.root / "repository.json", config) @@ -1727,11 +1771,15 @@ class MemberRepository: config["organization"] = organization write_json_atomic(self.root / "repository.json", config) - def preview_member_number(self, pattern: str | None = None) -> str: - selected_pattern = pattern or self.get_member_number_policy()["pattern"] + def preview_member_number( + self, pattern: str | None = None, allocation_strategy: str | None = None + ) -> str: + policy = self.get_member_number_policy() + selected_pattern = pattern or policy["pattern"] + selected_strategy = allocation_strategy or policy["allocation_strategy"] validate_member_number_pattern(selected_pattern) config = read_json(self.root / "repository.json") - return self._next_available_member_number(config, selected_pattern)[0] + return self._next_available_member_number(config, selected_pattern, selected_strategy)[0] def _member_directories(self) -> Iterable[Path]: if not self.members_root.exists(): @@ -1757,9 +1805,11 @@ class MemberRepository: raise RepositoryError("Ungültige Asset-ID.") return self.assets_root / asset_id - def _allocate_member_number(self, pattern: str) -> str: + def _allocate_member_number(self, pattern: str, allocation_strategy: str) -> str: config = read_json(self.root / "repository.json") - member_number, next_value = self._next_available_member_number(config, pattern) + member_number, next_value = self._next_available_member_number( + config, pattern, allocation_strategy + ) sequences = config.get("member_number_sequences") if not isinstance(sequences, dict): sequences = {} @@ -1768,15 +1818,20 @@ class MemberRepository: write_json_atomic(self.root / "repository.json", config) return member_number - def _next_available_member_number(self, config: dict, pattern: str) -> tuple[str, int]: - sequences = config.get("member_number_sequences") - if not isinstance(sequences, dict): - sequences = {} - try: - number = max(1, int(sequences.get(pattern, 1))) - except (TypeError, ValueError): - number = 1 + def _next_available_member_number( + self, config: dict, pattern: str, allocation_strategy: str + ) -> tuple[str, int]: + if allocation_strategy not in {"next_free", "max_plus_one"}: + raise RepositoryError("Ungültige Vergabestrategie für Mitgliedsnummern.") existing = {member.member_number.casefold() for member in self.list_members() if member.member_number} + number = 1 + if allocation_strategy == "max_plus_one": + used_values = ( + value + for member_number in existing + if (value := _number_from_formatted_member_number(pattern, member_number)) is not None + ) + number = max(used_values, default=0) + 1 for _attempt in range(1_000_000): candidate = format_member_number(pattern, number) number += 1 @@ -1784,6 +1839,33 @@ class MemberRepository: return candidate, number raise RepositoryError("Keine freie Mitgliedsnummer im konfigurierten Nummernbereich gefunden.") + @contextmanager + def _member_number_lock(self): + lock_path = self.root / ".member-number.lock" + lock_path.parent.mkdir(parents=True, exist_ok=True) + with lock_path.open("a+", encoding="utf-8") as handle: + if os.name == "nt": + import msvcrt + + handle.seek(0) + if not handle.read(1): + handle.write("\0") + handle.flush() + handle.seek(0) + msvcrt.locking(handle.fileno(), msvcrt.LK_LOCK, 1) + else: + import fcntl + + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + try: + yield + finally: + if os.name == "nt": + handle.seek(0) + msvcrt.locking(handle.fileno(), msvcrt.LK_UNLCK, 1) + else: + fcntl.flock(handle.fileno(), fcntl.LOCK_UN) + def _assert_member_number_available( self, member_number: str, @@ -1941,6 +2023,26 @@ def validate_member_number_pattern(pattern: str) -> None: raise RepositoryError(f"Ungültiges Mitgliedsnummern-Pattern: {exc}") from exc +def _number_from_formatted_member_number(pattern: str, member_number: str) -> int | None: + parts: list[str] = [] + number_groups = 0 + for literal, field_name, _format_spec, _conversion in Formatter().parse(pattern.strip()): + parts.append(re.escape(literal)) + if field_name == "year": + parts.append(re.escape(str(date.today().year))) + elif field_name == "number": + parts.append(r"(\d+)") + number_groups += 1 + match = re.fullmatch("".join(parts), member_number, flags=re.IGNORECASE) + if not match or not number_groups: + return None + values = [int(value) for value in match.groups()] + if any(value != values[0] for value in values[1:]): + return None + value = values[0] + return value if format_member_number(pattern, value).casefold() == member_number.casefold() else None + + def format_member_number(pattern: str, number: int, *, year: int | None = None) -> str: try: value = pattern.strip().format(number=number, year=year or date.today().year) diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index 5d6f402..1d6c640 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -47,6 +47,7 @@ class OptionsDialog(tk.Toplevel): number_policy = repository.get_member_number_policy() self.manual_numbers_var = tk.BooleanVar(value=number_policy["mode"] == "manual") self.number_pattern_var = tk.StringVar(value=number_policy["pattern"]) + self.number_strategy_var = tk.StringVar(value=number_policy["allocation_strategy"]) self.number_preview_var = tk.StringVar() organization = repository.get_configuration().get("organization") or {} if not isinstance(organization, dict): @@ -280,19 +281,38 @@ class OptionsDialog(tk.Toplevel): text="Platzhalter: {number}, {number:04d} und optional {year}", style="Muted.TLabel", ).grid(row=2, column=1, sticky="w", pady=(0, 12)) - ttk.Label(parent, text="Nächste Nummer").grid(row=3, column=0, sticky="w", padx=(0, 12), pady=6) + ttk.Label(parent, text="Vergabestrategie").grid( + row=3, column=0, sticky="nw", padx=(0, 12), pady=6 + ) + strategy_frame = ttk.Frame(parent) + strategy_frame.grid(row=3, column=1, sticky="w", pady=6) + ttk.Radiobutton( + strategy_frame, + text="Lücken füllen (nächste freie Nummer)", + variable=self.number_strategy_var, + value="next_free", + command=self._update_number_preview, + ).pack(anchor="w") + ttk.Radiobutton( + strategy_frame, + text="Immer Max++ (höchste Nummer + 1)", + variable=self.number_strategy_var, + value="max_plus_one", + command=self._update_number_preview, + ).pack(anchor="w", pady=(4, 0)) + ttk.Label(parent, text="Nächste Nummer").grid(row=4, column=0, sticky="w", padx=(0, 12), pady=6) ttk.Label(parent, textvariable=self.number_preview_var, style="TimelineHeader.TLabel").grid( - row=3, column=1, sticky="w", pady=6 + row=4, column=1, sticky="w", pady=6 ) ttk.Label( parent, text=( - "Der Zähler wird pro Pattern im Mitglieder-Store geführt. Bereits vergebene Nummern " - "werden niemals doppelt erzeugt." + "Die Strategie wird im Mitglieder-Store gespeichert. Die Vergabe ist gesperrt, " + "solange ein neues Mitglied angelegt wird, damit keine Nummer doppelt entsteht." ), style="Muted.TLabel", wraplength=620, - ).grid(row=4, column=0, columnspan=2, sticky="w", pady=(14, 0)) + ).grid(row=5, column=0, columnspan=2, sticky="w", pady=(14, 0)) self.number_pattern_var.trace_add("write", lambda *_args: self._update_number_preview()) self._update_member_number_controls() @@ -309,7 +329,9 @@ class OptionsDialog(tk.Toplevel): return pattern = self.number_pattern_var.get() try: - preview = self.repository.preview_member_number(pattern) + preview = self.repository.preview_member_number( + pattern, self.number_strategy_var.get() + ) except RepositoryError as exc: preview = f"UNGÜLTIG: {exc}" self.number_preview_var.set(preview) @@ -409,7 +431,11 @@ class OptionsDialog(tk.Toplevel): ) try: self.config_obj.save() - self.repository.save_member_number_policy(mode=number_mode, pattern=number_pattern) + self.repository.save_member_number_policy( + mode=number_mode, + pattern=number_pattern, + allocation_strategy=self.number_strategy_var.get(), + ) self.repository.save_organization( {key: variable.get() for key, variable in self.organization_vars.items()} ) diff --git a/tests/test_repository.py b/tests/test_repository.py index 61cf11a..9113779 100644 --- a/tests/test_repository.py +++ b/tests/test_repository.py @@ -1,4 +1,5 @@ import json +from concurrent.futures import ThreadPoolExecutor from decimal import Decimal import pytest @@ -184,6 +185,43 @@ def test_custom_pattern_and_manual_mode(tmp_path) -> None: repository.create_member(first_name="Duplicate", last_name="Member", member_number="special-7") +def test_member_number_strategy_fills_gaps_or_uses_max_plus_one(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + repository.create_member(first_name="One", last_name="Member", member_number="MA-001") + repository.create_member(first_name="Three", last_name="Member", member_number="MA-003") + + repository.save_member_number_policy( + mode="automatic", + pattern="MA-{number:03d}", + allocation_strategy="next_free", + ) + assert repository.preview_member_number() == "MA-002" + assert repository.get_member_number_policy()["allocation_strategy"] == "next_free" + + repository.save_member_number_policy( + mode="automatic", + pattern="MA-{number:03d}", + allocation_strategy="max_plus_one", + ) + assert repository.preview_member_number() == "MA-004" + created = repository.create_member(first_name="Four", last_name="Member") + assert created.member_number == "MA-004" + + +def test_concurrent_automatic_member_numbers_are_unique(tmp_path) -> None: + MemberRepository(tmp_path).initialize() + + def create(index: int) -> str: + repository = MemberRepository(tmp_path) + return repository.create_member(first_name=f"Member {index}", last_name="Concurrent").member_number + + with ThreadPoolExecutor(max_workers=8) as executor: + numbers = list(executor.map(create, range(16))) + + assert len(numbers) == len(set(numbers)) == 16 + + @pytest.mark.parametrize("pattern", ["", "CCMA-{year}", "{unknown}-{number}", "{number!r}"]) def test_invalid_member_number_patterns_are_rejected(pattern) -> None: with pytest.raises(RepositoryError):