From 10786cbad82192926e1b687e71d0fa92ced892f9 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:37:22 +0200 Subject: [PATCH] Move hook into storage.Collection The hook is only valid for filesystem storage, it's meaningless for other backends like databases. --- radicale/__init__.py | 38 +++++++++++--------------------------- radicale/storage.py | 19 +++++++++++++++---- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/radicale/__init__.py b/radicale/__init__.py index 0bc8d1d..6fde141 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -32,11 +32,9 @@ import itertools import os import posixpath import pprint -import shlex import socket import socketserver import ssl -import subprocess import threading import wsgiref.simple_server import zlib @@ -285,12 +283,12 @@ class Application: if user and is_authenticated: principal_path = "/%s/" % user if self.authorized(user, principal_path, "w"): - with self._lock_collection("r", user): + with self.Collection.acquire_lock("r", user): principal = next( self.Collection.discover(principal_path, depth="1"), None) if not principal: - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): self.Collection.create_collection(principal_path) # Verify content length @@ -363,20 +361,6 @@ class Application: allowed |= self.authorized(user, parent_path, permission) return allowed - @contextmanager - def _lock_collection(self, lock_mode, user): - """Lock the collection with ``permission`` and execute hook.""" - with self.Collection.acquire_lock(lock_mode) as value: - yield value - hook = self.configuration.get("storage", "hook") - if lock_mode == "w" and hook: - self.logger.debug("Running hook") - folder = os.path.expanduser(self.configuration.get( - "storage", "filesystem_folder")) - subprocess.check_call( - hook % {"user": shlex.quote(user or "Anonymous")}, - shell=True, cwd=folder) - def _read_content(self, environ): content_length = int(environ.get("CONTENT_LENGTH") or 0) if content_length > 0: @@ -391,7 +375,7 @@ class Application: """Manage DELETE request.""" if not self._access(user, path, "w"): return NOT_ALLOWED - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if not self._access(user, path, "w", item): return NOT_ALLOWED @@ -416,7 +400,7 @@ class Application: return client.OK, headers, answer if not self._access(user, path, "r"): return NOT_ALLOWED - with self._lock_collection("r", user): + with self.Collection.acquire_lock("r", user): item = next(self.Collection.discover(path), None) if not self._access(user, path, "r", item): return NOT_ALLOWED @@ -445,7 +429,7 @@ class Application: if not self.authorized(user, path, "w"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if item: return client.CONFLICT, {}, None @@ -461,7 +445,7 @@ class Application: if not self.authorized(user, path, "w"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if item: return client.CONFLICT, {}, None @@ -481,7 +465,7 @@ class Application: if not self._access(user, to_path, "w"): return NOT_ALLOWED - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if not self._access(user, path, "w", item): return NOT_ALLOWED @@ -519,7 +503,7 @@ class Application: if not self._access(user, path, "r"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("r", user): + with self.Collection.acquire_lock("r", user): items = self.Collection.discover( path, environ.get("HTTP_DEPTH", "0")) # take root item for rights checking @@ -544,7 +528,7 @@ class Application: if not self.authorized(user, path, "w"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if not isinstance(item, self.Collection): return client.CONFLICT, {}, None @@ -557,7 +541,7 @@ class Application: if not self._access(user, path, "w"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("w", user): + with self.Collection.acquire_lock("w", user): parent_path = storage.sanitize_path( "/%s/" % posixpath.dirname(path.strip("/"))) item = next(self.Collection.discover(path), None) @@ -612,7 +596,7 @@ class Application: if not self._access(user, path, "w"): return NOT_ALLOWED content = self._read_content(environ) - with self._lock_collection("r", user): + with self.Collection.acquire_lock("r", user): item = next(self.Collection.discover(path), None) if not self._access(user, path, "w", item): return NOT_ALLOWED diff --git a/radicale/storage.py b/radicale/storage.py index 102bfc1..2c28455 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -29,7 +29,9 @@ import errno import json import os import posixpath +import shlex import stat +import subprocess import threading import time from contextlib import contextmanager @@ -371,12 +373,14 @@ class BaseCollection: @classmethod @contextmanager - def acquire_lock(cls, mode): + def acquire_lock(cls, mode, user=None): """Set a context manager to lock the whole storage. ``mode`` must either be "r" for shared access or "w" for exclusive access. + ``user`` is the name of the logged in user or empty. + """ raise NotImplementedError @@ -743,13 +747,15 @@ class Collection(BaseCollection): @classmethod @contextmanager - def acquire_lock(cls, mode): + def acquire_lock(cls, mode, user=None): def condition(): if mode == "r": return not cls._writer else: return not cls._writer and cls._readers == 0 + folder = os.path.expanduser(cls.configuration.get( + "storage", "filesystem_folder")) # Use a primitive lock which only works within one process as a # precondition for inter-process file-based locking with cls._lock: @@ -770,8 +776,6 @@ class Collection(BaseCollection): else: cls._writer = True if not cls._lock_file: - folder = os.path.expanduser( - cls.configuration.get("storage", "filesystem_folder")) cls._makedirs_synced(folder) lock_path = os.path.join(folder, ".Radicale.lock") cls._lock_file = open(lock_path, "w+") @@ -797,6 +801,13 @@ class Collection(BaseCollection): cls._lock_file_locked = True try: yield + # execute hook + hook = cls.configuration.get("storage", "hook") + if mode == "w" and hook: + cls.logger.debug("Running hook") + subprocess.check_call( + hook % {"user": shlex.quote(user or "Anonymous")}, + shell=True, cwd=folder) finally: with cls._lock: if mode == "r":