diff --git a/src/ccma/assets/CHANGELOG.json b/src/ccma/assets/CHANGELOG.json index 21e3df5..0c8f51c 100644 --- a/src/ccma/assets/CHANGELOG.json +++ b/src/ccma/assets/CHANGELOG.json @@ -14,7 +14,8 @@ "Mitglieder mit offenen Hausmeister-Vorgängen werden in der Liste je nach höchster Dringlichkeit blau, gelb oder rot hervorgehoben.", "Offene Forderungen von Mitgliedern mit aktivem Lastschriftmandat können für einen frei wählbaren Fälligkeitszeitraum gesammelt und als bankfähige SEPA-XML-Datei oder als CSV-Übersicht exportiert werden; unvollständige Mandate werden dabei verständlich ausgewiesen.", "Für geplante SEPA-Einzüge können personalisierte, mit Thunderbird kompatible E-Mail-Entwürfe erzeugt werden. Die Mitteilungen enthalten Betrag, Einzugsdatum und Mandatsdaten und werden automatisch in der jeweiligen Mitgliederakte archiviert.", - "Mahnungsentwürfe können direkt als personalisierte, mit Thunderbird kompatible E-Mail-Datei ausgegeben und in der Mitgliederakte archiviert werden; dabei werden der Versand verbucht sowie Zahlungsfrist und gegebenenfalls Mahngebühr wirksam." + "Mahnungsentwürfe können direkt als personalisierte, mit Thunderbird kompatible E-Mail-Datei ausgegeben und in der Mitgliederakte archiviert werden; dabei werden der Versand verbucht sowie Zahlungsfrist und gegebenenfalls Mahngebühr wirksam.", + "Für automatisch vergebene Mitgliedsnummern kann gewählt werden, ob vorhandene Lücken mit der nächsten freien Nummer gefüllt werden oder stets die höchste bestehende Nummer um eins erhöht wird; die Vergabe ist gegen parallele Doppelbelegungen abgesichert." ] }, { 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):