Retry with random filename if UID is rejected.

see #173
This commit is contained in:
Markus Unterwaditzer 2015-01-30 15:11:37 +01:00
parent 0eb69a810f
commit 4ddcb0fef4
3 changed files with 29 additions and 21 deletions

View file

@ -28,18 +28,10 @@ event.
The filename *should* consist of the ``ident``, followed by the file extension. The filename *should* consist of the ``ident``, followed by the file extension.
The ``ident`` is either the ``UID``, if the item has one, else a string with The ``ident`` is either the ``UID``, if the item has one, else a string with
similar properties as the ``UID``: similar properties as the ``UID``. However, several restrictions of the
underlying filesystem might make an implementation of this naming scheme for
Type name: UID items' filenames impossible. The approach to deal with such cases is left to
the client, which are free to choose a different scheme for filenames instead.
Type purpose: To specify a value that represents a globally unique
identifier corresponding to the individual or resource associated
with the vCard.
-- The vCard_ RFC
Character limitations imposed by the filesystem *should* be circumvented by
replacing the offending characters with underscores ``_``.
.. _vCard: https://tools.ietf.org/html/rfc6350 .. _vCard: https://tools.ietf.org/html/rfc6350
.. _iCalendar: https://tools.ietf.org/html/rfc5545 .. _iCalendar: https://tools.ietf.org/html/rfc5545

View file

@ -49,3 +49,9 @@ class TestFilesystemStorage(StorageTests):
s.upload(Item(u'UID:a/b/c')) s.upload(Item(u'UID:a/b/c'))
item_file, = tmpdir.listdir() item_file, = tmpdir.listdir()
assert str(item_file).endswith('a_b_c.txt') assert str(item_file).endswith('a_b_c.txt')
def test_too_long_uid(self, tmpdir):
s = self.storage_class(str(tmpdir), '.txt')
item = Item(u'UID:' + u'hue' * 600)
href, etag = s.upload(item)
assert item.uid not in href

View file

@ -2,6 +2,7 @@
import errno import errno
import os import os
import uuid
from atomicwrites import atomic_write from atomicwrites import atomic_write
@ -70,11 +71,14 @@ class FilesystemStorage(Storage):
def _get_filepath(self, href): def _get_filepath(self, href):
return os.path.join(self.path, href) return os.path.join(self.path, href)
def _get_href(self, item): def _deterministic_href(self, item):
# XXX: POSIX only defines / and \0 as invalid chars, but we should make # XXX: POSIX only defines / and \0 as invalid chars, but we should make
# this work crossplatform. # this work crossplatform.
return item.ident.replace('/', '_') + self.fileext return item.ident.replace('/', '_') + self.fileext
def _random_href(self):
return str(uuid.uuid4()) + self.fileext
def list(self): def list(self):
for fname in os.listdir(self.path): for fname in os.listdir(self.path):
fpath = os.path.join(self.path, fname) fpath = os.path.join(self.path, fname)
@ -88,25 +92,34 @@ class FilesystemStorage(Storage):
return (Item(f.read().decode(self.encoding)), return (Item(f.read().decode(self.encoding)),
get_etag_from_file(fpath)) get_etag_from_file(fpath))
except IOError as e: except IOError as e:
import errno
if e.errno == errno.ENOENT: if e.errno == errno.ENOENT:
raise exceptions.NotFoundError(href) raise exceptions.NotFoundError(href)
else: else:
raise raise
def upload(self, item): def upload(self, item):
href = self._get_href(item)
fpath = self._get_filepath(href)
if not isinstance(item.raw, text_type): if not isinstance(item.raw, text_type):
raise TypeError('item.raw must be a unicode string.') raise TypeError('item.raw must be a unicode string.')
try:
href = self._deterministic_href(item)
return self._upload_impl(item, href)
except OSError as e:
if e.errno == errno.ENAMETOOLONG:
logger.debug('UID as filename rejected, trying with random '
'one.')
href = self._random_href()
return self._upload_impl(item, href)
else:
raise
def _upload_impl(self, item, href):
fpath = self._get_filepath(href)
try: try:
with atomic_write(fpath, mode='wb', overwrite=False) as f: with atomic_write(fpath, mode='wb', overwrite=False) as f:
f.write(item.raw.encode(self.encoding)) f.write(item.raw.encode(self.encoding))
return href, get_etag_from_fileobject(f) return href, get_etag_from_fileobject(f)
except OSError as e: except OSError as e:
import errno
if e.errno == errno.EEXIST: if e.errno == errno.EEXIST:
raise exceptions.AlreadyExistingError(item) raise exceptions.AlreadyExistingError(item)
else: else:
@ -114,9 +127,6 @@ class FilesystemStorage(Storage):
def update(self, href, item, etag): def update(self, href, item, etag):
fpath = self._get_filepath(href) fpath = self._get_filepath(href)
if href != self._get_href(item) and item.uid:
logger.warning('href != uid + fileext: href={}; uid={}'
.format(href, item.uid))
if not os.path.exists(fpath): if not os.path.exists(fpath):
raise exceptions.NotFoundError(item.uid) raise exceptions.NotFoundError(item.uid)
actual_etag = get_etag_from_file(fpath) actual_etag = get_etag_from_file(fpath)