Polish design, move blob redirections to app level

6 files changed, 31 insertions, 185 deletions

diff --git a/active_document/__init__.py b/active_document/__init__.py
index 90de81a..afa733e 100644
--- a/active_document/__init__.py
+++ b/active_document/__init__.py
@@ -20,7 +20,7 @@ from active_document.env import ACCESS_CREATE, ACCESS_WRITE, ACCESS_READ, \
         ACCESS_LEVELS, ACCESS_SYSTEM, ACCESS_LOCAL, ACCESS_REMOTE, \
         index_flush_timeout, index_flush_threshold, \
         index_write_queue, \
-        NotFound, NotModified, Forbidden, Redirect, Seqno, DEFAULT_LANG, \
+        NotFound, NotModified, Forbidden, Seqno, DEFAULT_LANG, \
         uuid, default_lang
 
 from active_document.metadata import Metadata, PropertyMeta, Property, \
diff --git a/active_document/commands.py b/active_document/commands.py
index 4ab8587..2a0d1b1 100644
--- a/active_document/commands.py
+++ b/active_document/commands.py
@@ -95,10 +95,10 @@ class Request(dict):
     if_modified_since = None
     response = None
 
-    def __init__(self, *args, **props):
-        if args:
-            props = args[0]
-        dict.__init__(self, props)
+    def __init__(self, props_=None, **kwargs):
+        if props_ is not None:
+            kwargs = props_
+        dict.__init__(self, kwargs)
         self._pos = 0
 
     @property
@@ -195,13 +195,15 @@ class CommandsProcessor(object):
     def super_call(self, request, response):
         raise CommandNotFound()
 
-    def call(self, request, response):
+    def call(self, request, response=None):
         cmd = self.resolve(request)
         enforce(cmd is not None, CommandNotFound, 'Unsupported command')
 
         enforce(request.access_level & cmd.access_level, env.Forbidden,
                 'Operation is permitted on requester\'s level')
 
+        if response is None:
+            response = Response()
         request.commands = self
         request.response = response
 
diff --git a/active_document/env.py b/active_document/env.py
index 6b42728..4db3e3d 100644
--- a/active_document/env.py
+++ b/active_document/env.py
@@ -105,13 +105,6 @@ class Forbidden(Exception):
     pass
 
 
-class Redirect(Exception):
-
-    def __init__(self, location, *args, **kwargs):
-        self.location = location
-        Exception.__init__(self, *args, **kwargs)
-
-
 class NotModified(Exception):
     pass
 
diff --git a/active_document/metadata.py b/active_document/metadata.py
index e712663..ec59a5b 100644
--- a/active_document/metadata.py
+++ b/active_document/metadata.py
@@ -119,31 +119,6 @@ class PropertyMeta(dict):
             meta['mtime'] = os.stat(path_).st_mtime
         dict.__init__(self, meta)
 
-    def url(self, part=None, default=None, prefix=None):
-
-        def compose_url(url):
-            if url is None:
-                url = default
-            if prefix and url.startswith('/'):
-                url = prefix + url
-            return url
-
-        url = self.get('url')
-        if url is None or isinstance(url, basestring):
-            return compose_url(url)
-
-        if part is not None:
-            file_meta = url.get(part)
-            enforce(file_meta and 'url' in file_meta,
-                    env.NotFound, 'No BLOB for %r', part)
-            return compose_url(file_meta['url'])
-
-        result = []
-        for i in sorted(url.values(), cmp=lambda x, y:
-                cmp(x.get('order'), y.get('order'))):
-            result.append(compose_url(i['url']))
-        return result
-
     @classmethod
     def is_blob(cls, blob):
         return isinstance(blob, (type(None), basestring, cls)) or \
diff --git a/active_document/volume.py b/active_document/volume.py
index 5da9f44..da897b1 100644
--- a/active_document/volume.py
+++ b/active_document/volume.py
@@ -17,7 +17,7 @@ import os
 import time
 import logging
 from contextlib import contextmanager
-from os.path import exists, join, abspath, isdir
+from os.path import exists, join, abspath
 
 from active_document import env
 from active_document.directory import Directory
@@ -26,8 +26,9 @@ from active_document.commands import document_command, directory_command
 from active_document.commands import CommandsProcessor, property_command
 from active_document.commands import to_int, to_list
 from active_document.metadata import BlobProperty, BrowsableProperty
+from active_document.metadata import StoredProperty
 from active_document.metadata import PropertyMeta
-from active_toolkit import coroutine, util, sockets, enforce
+from active_toolkit import coroutine, util, enforce
 
 
 _logger = logging.getLogger('active_document.volume')
@@ -207,31 +208,14 @@ class VolumeCommands(CommandsProcessor):
 
         prop.assert_access(env.ACCESS_READ)
 
-        if not isinstance(prop, BlobProperty):
+        if isinstance(prop, StoredProperty):
             value = doc.get(prop.name, request.accept_language or self._lang)
             return prop.on_get(doc, value)
-
-        meta = prop.on_get(doc, meta)
-        enforce(meta is not None, env.NotFound, 'BLOB does not exist')
-
-        url = meta.url(part)
-        if url is not None:
-            if not isinstance(url, basestring):
-                return url
-            raise env.Redirect(url)
-
-        enforce('path' in meta, env.NotFound, 'BLOB does not exist')
-
-        path = meta['path']
-        if isdir(path):
-            dir_info, dir_reader = sockets.encode_directory(path)
-            response.content_length = dir_info.content_length
-            response.content_type = dir_info.content_type
-            return dir_reader
         else:
-            response.content_length = os.stat(path).st_size
-            response.content_type = prop.mime_type
-            return _file_reader(path)
+            meta = prop.on_get(doc, meta)
+            enforce(meta is not None and ('path' in meta or 'url' in meta),
+                    env.NotFound, 'BLOB does not exist')
+            return meta
 
     def before_create(self, request, props):
         ts = int(time.time())
@@ -290,12 +274,3 @@ class VolumeCommands(CommandsProcessor):
         for prop in request['reply']:
             result[prop] = metadata[prop].on_get(doc, doc.get(prop, lang))
         return result
-
-
-def _file_reader(path):
-    with file(path, 'rb') as f:
-        while True:
-            chunk = f.read(sockets.BUFFER_SIZE)
-            if not chunk:
-                break
-            yield chunk
diff --git a/tests/units/volume.py b/tests/units/volume.py
index ffde031..d501430 100755
--- a/tests/units/volume.py
+++ b/tests/units/volume.py
@@ -18,7 +18,7 @@ from __init__ import tests
 import active_document as ad
 from active_document import env, document, SingleVolume, \
         Request, Response, Document, active_property, \
-        BlobProperty, NotFound, Redirect
+        BlobProperty, NotFound
 from active_document.volume import VolumeCommands
 from active_toolkit import sockets, coroutine
 
@@ -145,12 +145,10 @@ class VolumeTest(tests.Test):
         self.assertRaises(RuntimeError, self.call, 'PUT', document='testdocument', guid=guid, prop='blob', content={'path': '/'})
 
         self.call('PUT', document='testdocument', guid=guid, prop='blob', content='blob1')
-        stream = self.call('GET', document='testdocument', guid=guid, prop='blob')
-        self.assertEqual('blob1', ''.join([i for i in stream]))
+        self.assertEqual('blob1', file(self.call('GET', document='testdocument', guid=guid, prop='blob')['path']).read())
 
         self.call('PUT', document='testdocument', guid=guid, prop='blob', content_stream=StringIO('blob2'))
-        stream = self.call('GET', document='testdocument', guid=guid, prop='blob')
-        self.assertEqual('blob2', ''.join([i for i in stream]))
+        self.assertEqual('blob2', file(self.call('GET', document='testdocument', guid=guid, prop='blob')['path']).read())
 
         self.call('PUT', document='testdocument', guid=guid, prop='blob', content=None)
         self.assertRaises(NotFound, self.call, 'GET', document='testdocument', guid=guid, prop='blob')
@@ -176,8 +174,7 @@ class VolumeTest(tests.Test):
                 'mtime': os.stat(blob_path).st_mtime,
                 }
 
-        stream = self.call('GET', document='testdocument', guid=guid, prop='blob')
-        self.assertEqual('blob', ''.join([i for i in stream]))
+        self.assertEqual('blob', file(self.call('GET', document='testdocument', guid=guid, prop='blob')['path']).read())
 
         self.assertEqual(
                 {'guid': guid, 'blob': blob_meta},
@@ -188,49 +185,6 @@ class VolumeTest(tests.Test):
             ],
             self.call('GET', document='testdocument', reply=['guid', 'blob'])['result'])
 
-    def test_CommandsGetBlobDirectory(self):
-
-        class TestDocument(Document):
-
-            @active_property(slot=1, default='')
-            def prop(self, value):
-                return value
-
-            @active_property(BlobProperty)
-            def blob(self, value):
-                return value
-
-            @active_property(prefix='L', localized=True, default='')
-            def localized_prop(self, value):
-                return value
-
-        self.volume = SingleVolume(tests.tmpdir, [TestDocument])
-        guid = self.call('POST', document='testdocument', content={})
-
-        blob_path = tests.tmpdir + '/testdocument/%s/%s/blob' % (guid[:2], guid)
-        self.touch((blob_path, '{}'))
-        self.touch((blob_path + '.blob/1/2/3', 'a'))
-        self.touch((blob_path + '.blob/4/5', 'b'))
-        self.touch((blob_path + '.blob/6', 'c'))
-
-        stream = StringIO()
-        for chunk in self.call('GET', document='testdocument', guid=guid, prop='blob'):
-            stream.write(chunk)
-        stream.seek(0)
-
-        msg = Message()
-        msg['content-type'] = self.response.content_type
-
-        files = sockets.decode_multipart(stream, self.response.content_length,
-                msg.get_boundary())
-        self.assertEqual(
-                sorted([
-                    ('1/2/3', 'a'),
-                    ('4/5', 'b'),
-                    ('6', 'c'),
-                    ]),
-                sorted([(name, content.read()) for name, content in files]))
-
     def test_CommandsGetAbsentBlobs(self):
 
         class TestDocument(Document):
@@ -515,49 +469,9 @@ class VolumeTest(tests.Test):
         guid = self.call('POST', document='testdocument', content={})
         self.call('PUT', document='testdocument', guid=guid, prop='blob', url='http://sugarlabs.org')
 
-        try:
-            self.call('GET', document='testdocument', guid=guid, prop='blob')
-            assert False
-        except Redirect, redirect:
-            self.assertEqual('http://sugarlabs.org', redirect.location)
-
-    def test_CompositeBlobs(self):
-
-        class TestDocument(Document):
-
-            @active_property(slot=1, default='')
-            def prop(self, value):
-                return value
-
-            @active_property(BlobProperty)
-            def blob(self, value):
-                return value
-
-            @active_property(prefix='L', localized=True, default='')
-            def localized_prop(self, value):
-                return value
-
-        self.volume = SingleVolume(tests.tmpdir, [TestDocument])
-        guid = self.call('POST', document='testdocument', content={})
-        self.call('PUT', document='testdocument', guid=guid, prop='blob', url={
-            'file3': {'order': 3, 'url': 'url3', 'foo': 'bar'},
-            'file2': {'order': 2, 'url': 'url2', 'probe': None},
-            'file1': {'order': 1, 'url': 'url1'},
-            })
-
-        self.assertEqual([
-            'url1',
-            'url2',
-            'url3',
-            ],
-            self.call('GET', document='testdocument', guid=guid, prop='blob'))
-        self.assertRaises(env.NotFound,
-                self.call, 'GET', document='testdocument', guid=guid, prop='blob', part='fake')
-        try:
-            self.call('GET', document='testdocument', guid=guid, prop='blob', part='file2')
-            assert False
-        except Redirect, redirect:
-            self.assertEqual('url2', redirect.location)
+        self.assertEqual(
+                'http://sugarlabs.org',
+                self.call('GET', document='testdocument', guid=guid, prop='blob')['url'])
 
     def test_before_create(self):
 
@@ -844,18 +758,14 @@ class VolumeTest(tests.Test):
                 meta['path'] = 'new-blob'
                 return meta
 
-            @active_property(BlobProperty)
-            def empty_blob(self, meta):
-                return ad.PropertyMeta(url='http://sugarlabs.org')
-
         self.volume = SingleVolume(tests.tmpdir, [TestDocument])
         guid = self.call('POST', document='testdocument', content={})
         self.touch(('new-blob', 'new-blob'))
         self.call('PUT', document='testdocument', guid=guid, prop='blob', content='old-blob')
 
         self.assertEqual(
-                ['new-blob'],
-                [i for i in self.call('GET', document='testdocument', guid=guid, prop='blob')])
+                'new-blob',
+                self.call('GET', document='testdocument', guid=guid, prop='blob')['path'])
         self.assertEqual(
                 '1',
                 self.call('GET', document='testdocument', guid=guid, prop='prop1'))
@@ -866,12 +776,6 @@ class VolumeTest(tests.Test):
                 {'prop1': '1', 'prop2': -1},
                 self.call('GET', document='testdocument', guid=guid, reply=['prop1', 'prop2']))
 
-        try:
-            self.call('GET', document='testdocument', guid=guid, prop='empty_blob')
-            assert False
-        except Redirect, redirect:
-            self.assertEqual('http://sugarlabs.org', redirect.location)
-
     def test_properties_OverrideSet(self):
 
         class TestDocument(Document):
@@ -915,16 +819,13 @@ class VolumeTest(tests.Test):
         self.assertRaises(NotFound, self.call, 'GET', document='testdocument', guid=guid, prop='blob1')
 
         self.call('PUT', document='testdocument', guid=guid, prop='blob1', content='blob2')
-        try:
-            self.call('GET', document='testdocument', guid=guid, prop='blob1')
-            assert False
-        except Redirect, redirect:
-            self.assertEqual('blob2', redirect.location)
+        self.assertEqual('blob2', self.call('GET', document='testdocument', guid=guid, prop='blob1')['url'])
 
         guid = self.call('POST', document='testdocument', content={'blob2': 'foo'})
-        self.assertEqual(' foo ', ''.join(self.call('GET', document='testdocument', guid=guid, prop='blob2')))
+        self.assertEqual(' foo ', file(self.call('GET', document='testdocument', guid=guid, prop='blob2')['path']).read())
+
         self.call('PUT', document='testdocument', guid=guid, prop='blob2', content='bar')
-        self.assertEqual(' bar ', ''.join(self.call('GET', document='testdocument', guid=guid, prop='blob2')))
+        self.assertEqual(' bar ', file(self.call('GET', document='testdocument', guid=guid, prop='blob2')['path']).read())
 
     def test_SubCall(self):
 
@@ -950,11 +851,11 @@ class VolumeTest(tests.Test):
 
         guid = self.call('POST', document='testdocument', content={'blob': '0'})
         coroutine.dispatch()
-        self.assertEqual('0!', ''.join(self.call('GET', document='testdocument', guid=guid, prop='blob')))
+        self.assertEqual('0!', file(self.call('GET', document='testdocument', guid=guid, prop='blob')['path']).read())
 
         self.call('PUT', document='testdocument', guid=guid, prop='blob', content='1')
         coroutine.dispatch()
-        self.assertEqual('0!1!', ''.join(self.call('GET', document='testdocument', guid=guid, prop='blob')))
+        self.assertEqual('0!1!', file(self.call('GET', document='testdocument', guid=guid, prop='blob')['path']).read())
 
     def test_Group(self):