Remove the "info" attribute from ImageRequest (#408)

* img_request.py: create a replacement ImageRequest class

* img.py: starting using the new request class

* img_request.py: remember to import 'unquote'

* img_request.py: restore the 'request_path' property

* img_request.py: create the 'cache_path' property (renamed from as_path)

* webapp.py: don't store the ImageInfo on the ImageRequest class

* img_request.py: restore the 'request_resolution_too_large' method

* webapp.py: use the new signature for request_resolution_too_large()

* webapp.py: we use 'fmt' instead of 'format', as the latter is a keyword

* webapp.py: pass the ImageInfo down into _make_image()

* img_request.py: make region_param() a method

* transforms.py: start plumbing ImageInfo into _derive_with_pil

* transforms.py: pass 'img_info' through the transform() method

* webapp.py: use the new transform() method

* img_request.py: make size_param() a method

* transforms.py: use the new size_param() method

* img_request.py: add rotation_param() as a method

* transforms.py: more bugfixes

* img_request.py: simplify some path methods

* img_request.py: bring back canonical_cache_path

* Plumb through img_info some more

* img_request.py: requests are immutable after creation!

* img_request.py: small bugfix

* Plumbing to support is_canonical()

* Pass information around for the canonical Link header

* transforms.py: plumbing through more info

* transform.py: that's a function, not an attribute

* transforms.py: clean up a few more .info instances

* Start fixing more broken tests

* More test fixes

* webapp.py: one final fix to get tests passing

* img_request.py: expand the docstring

* Remove a whole pile of now-unused code

* img_request_t.py: rearrange some test code

* img_request_t.py: don't forget that import!

* /s/fmt/format

* webapp.py: tidy up the signature of _set_canonical_link()

* transforms.py: consistent names in _scales_to_reduce_arg()

* Move ImageRequest back into img.py

* img_t.py: namespace correctly

* transform(): Use 'image_request' instead of 'img_request'

* transforms.py: more use of image_ instead of img_

* More naming things with img_ / image_

* A few more minor fixes and cleanups

* webapp.py: import fixes

* img_t.py: another import error

* parameters.py: make the naming consistent

* webapp.py: fill in that string interpolation

loris/img.py

@@ -7,203 +7,109 @@
 import errno
 from logging import getLogger
 from os import path
+import os
 
 try:
     from urllib.parse import quote_plus, unquote
 except ImportError:  # Python 2
     from urllib import quote_plus, unquote
 
-from loris.loris_exception import ImageException
+import attr
+
 from loris.parameters import RegionParameter, RotationParameter, SizeParameter
 from loris.utils import mkdir_p, safe_rename, symlink
 
 logger = getLogger(__name__)
 
+
+@attr.s(slots=True, frozen=True)
 class ImageRequest(object):
-    '''
-    Slots:
-        ident (str)
-        region_value (str):
-            copied exactly from the URI
-        size_value (str)
-            copied exactly from the URI
-        rotation_value (str)
-            copied exactly from the URI
-        quality (str)
-            copied exactly from the URI
-        format (str)
-            3 char string from the URI, (derived from) HTTP headers, or else the
-            default.
-        region_param (parameters.RegionParameter):
-            See RegionParameter.__slots__. The region is represented there as
-            both pixels and decmials.
-        size_param (parameters.SizeParameter)
-            See SizeParameter.__slots__.
-        rotation_param (parameters.RotationParameter)
-            See RotationParameter.__slots__.
-        info (ImageInfo):
-        is_canonical (bool):
-            True if this is a canonical path.
-        as_path (str):
-            Useful as a relative path from a cache root. Based on the original
-            request values.
-        canonical_as_path (str):
-            Useful as a relative path from a cache root. Based on values
-            normalized to the canonical request syntax.
-        request_path
-            Path of the request for tacking on to the service host and creating
-            a URI based on the original request.
-        canonical_request_path
-            Path of the request for tacking on to the service host and creating
-            a URI based on the normalized ('canonical') values.
-            ('canonical') values.
-        Raises:
+    """Stores information about a user's request for an image.
 
+    Specifically, it holds a slightly more convenient representation of the
+    request encoded in a IIIF URL:
 
-    '''
-    __slots__ = (
-        '_canonical_cache_path',
-        '_canonical_request_path',
-        '_cache_path',
-        '_info',
-        '_is_canonical',
-        '_region_param',
-        '_request_path',
-        '_rotation_param',
-        '_size_param',
-        'format',
-        'ident',
-        'quality',
-        'region_value',
-        'rotation_value',
-        'size_value'
-    )
-
-    def __init__(self, ident, region, size, rotation, quality, target_format):
-
-        self.ident, self.region_value, self.size_value = map(unquote, (ident, region, size))
-        self.rotation_value = rotation
-        self.quality = quality
-        self.format = target_format
-
-        logger.debug('region slice: %s', region)
-        logger.debug('size slice: %s', size)
-        logger.debug('rotation slice: %s', rotation)
-        logger.debug('quality slice: %s', self.quality)
-        logger.debug('format extension: %s', self.format)
-
-        # These aren't set until we first access them
-        self._canonical_cache_path = None
-        self._canonical_request_path = None
-        self._cache_path = None
-        self._request_path = None
-
-        self._is_canonical = None
-
-        self._region_param = None
-        self._rotation_param = None
-        self._size_param = None
-
-        # This is a little awkward. We may need it, but not right away (only if we're
-        # filling out the param slots), so the user (of the class) has to set
-        # it before accessing most of the above.
-        self._info = None
+        /{identifier}/{region}/{size}/{rotation}/{quality}.{format}
 
-    @property
-    def region_param(self):
-        if self._region_param is None:
-            self._region_param = RegionParameter(self.region_value, self.info)
-        return self._region_param
+    """
+    ident = attr.ib(converter=unquote)
+    region_value = attr.ib(converter=unquote)
+    size_value = attr.ib(converter=unquote)
+    rotation_value = attr.ib()
+    quality = attr.ib()
+    format = attr.ib()
 
     @property
-    def size_param(self):
-        if self._size_param is None:
-            self._size_param = SizeParameter(self.size_value, self.region_param)
-        return self._size_param
-
-    @property
-    def rotation_param(self):
-        if self._rotation_param is None:
-            self._rotation_param = RotationParameter(self.rotation_value)
-        return self._rotation_param
+    def cache_path(self):
+        path = os.path.join(
+            self.ident,
+            self.region_value,
+            self.size_value,
+            self.rotation_value,
+            self.quality
+        )
+        return '%s.%s' % (path, self.format)
+
+    def canonical_cache_path(self, image_info):
+        path = os.path.join(
+            self.ident,
+            self.region_param(image_info).canonical_uri_value,
+            self.size_param(image_info).canonical_uri_value,
+            self.rotation_param().canonical_uri_value,
+            self.quality
+        )
+        return '%s.%s' % (path, self.format)
+
+    def is_canonical(self, image_info):
+        return self.cache_path == self.canonical_cache_path(image_info)
 
     @property
     def request_path(self):
-        if self._request_path is None:
-            p = '/'.join((
-                quote_plus(self.ident),
-                self.region_value,
-                self.size_value,
-                self.rotation_value,
-                self.quality
-            ))
-            self._request_path = '%s.%s' % (p,self.format)
-        return self._request_path
+        path = os.path.join(
+            quote_plus(self.ident),
+            self.region_value,
+            self.size_value,
+            self.rotation_value,
+            self.quality
+        )
+        return '%s.%s' % (path, self.format)
+
+    def canonical_request_path(self, image_info):
+        path = os.path.join(
+            quote_plus(self.ident),
+            self.region_param(image_info).canonical_uri_value,
+            self.size_param(image_info).canonical_uri_value,
+            self.rotation_param().canonical_uri_value,
+            self.quality
+        )
+        return '%s.%s' % (path, self.format)
+
+    def region_param(self, image_info):
+        return RegionParameter(
+            uri_value=self.region_value,
+            image_info=image_info
+        )
+
+    def size_param(self, image_info):
+        return SizeParameter(
+            uri_value=self.size_value,
+            region_parameter=self.region_param(image_info)
+        )
 
-    @property
-    def canonical_request_path(self):
-        if self._canonical_request_path is None:
-            p = '/'.join((
-                quote_plus(self.ident),
-                self.region_param.canonical_uri_value,
-                self.size_param.canonical_uri_value,
-                self.rotation_param.canonical_uri_value,
-                self.quality
-            ))
-            self._canonical_request_path = '%s.%s' % (p,self.format)
-        return self._canonical_request_path
+    def rotation_param(self):
+        return RotationParameter(uri_value=self.rotation_value)
 
-    @property
-    def as_path(self):
-        if self._cache_path is None:
-            p = path.join(self.ident,
-                self.region_value,
-                self.size_value,
-                self.rotation_value,
-                self.quality
-            )
-            self._cache_path = '%s.%s' % (p, self.format)
-        return self._cache_path
+    def request_resolution_too_large(self, max_size_above_full, image_info):
+        if max_size_above_full == 0:
+            return False
 
-    @property
-    def canonical_as_path(self):
-        if self._canonical_cache_path is None:
-            p = path.join(self.ident,
-                self.region_param.canonical_uri_value,
-                self.size_param.canonical_uri_value,
-                self.rotation_param.canonical_uri_value,
-                self.quality
-            )
-            self._canonical_cache_path = '%s.%s' % (p, self.format)
-        return self._canonical_cache_path
+        region_param = self.region_param(image_info=image_info)
+        size_param = self.size_param(image_info=image_info)
 
-    @property
-    def is_canonical(self):
-        if self._is_canonical is None:
-            self._is_canonical = (self.as_path == self.canonical_as_path)
-        return self._is_canonical
+        max_width = region_param.pixel_w * max_size_above_full / 100
+        max_height = region_param.pixel_h * max_size_above_full / 100
 
-    @property
-    def info(self):
-        if self._info is None:
-            # For dev purposes only. This should never happen.
-            raise ImageException('Image.info not set!')
-        else:
-            return self._info
-
-    @info.setter
-    def info(self, i):
-        self._info = i
-
-    def request_resolution_too_large(self, max_size_above_full):
-        if max_size_above_full == 0:
-            return False
-        max_width = self.region_param.pixel_w * max_size_above_full / 100
-        max_height = self.region_param.pixel_h * max_size_above_full / 100
-        if self.size_param.w > max_width or \
-                self.size_param.h > max_height:
-            return True
-        return False
+        return (size_param.w > max_width) or (size_param.h > max_height)
 
 
 class ImageCache(dict):
@@ -226,7 +132,7 @@ def __getitem__(self, image_request):
             else:
                 raise
 
-    def __setitem__(self, image_request, canonical_fp):
+    def store(self, image_request, image_info, canonical_fp):
         # Because we're working with files, it's more practical to put derived
         # images where the cache expects them when they are created (i.e. by
         # Loris#_make_image()), so __setitem__, as defined by the dict API
@@ -241,7 +147,7 @@ def __setitem__(self, image_request, canonical_fp):
         # So: when Loris#_make_image is called, it gets a path from
         # ImageCache#get_canonical_cache_path and passes that to the
         # transformer.
-        if not image_request.is_canonical:
+        if not image_request.is_canonical(image_info):
             requested_fp = self.get_request_cache_path(image_request)
             symlink(src=canonical_fp, dst=requested_fp)
 
@@ -259,20 +165,23 @@ def get(self, image_request):
             return None
 
     def get_request_cache_path(self, image_request):
-        request_fp = image_request.as_path
+        request_fp = image_request.cache_path
         return path.realpath(path.join(self.cache_root, unquote(request_fp)))
 
-    def get_canonical_cache_path(self, image_request):
-        canonical_fp = image_request.canonical_as_path
+    def get_canonical_cache_path(self, image_request, image_info):
+        canonical_fp = image_request.canonical_cache_path(image_info=image_info)
         return path.realpath(path.join(self.cache_root, unquote(canonical_fp)))
 
-    def create_dir_and_return_file_path(self, image_request):
-        target_fp = self.get_canonical_cache_path(image_request)
+    def create_dir_and_return_file_path(self, image_request, image_info):
+        target_fp = self.get_canonical_cache_path(image_request, image_info)
         target_dp = path.dirname(target_fp)
         mkdir_p(target_dp)
         return target_fp
 
-    def upsert(self, image_request, temp_fp):
-        target_fp = self.create_dir_and_return_file_path(image_request)
+    def upsert(self, image_request, temp_fp, image_info):
+        target_fp = self.create_dir_and_return_file_path(
+            image_request=image_request,
+            image_info=image_info
+        )
         safe_rename(temp_fp, target_fp)
         return target_fp

loris/loris_exception.py

@@ -13,10 +13,6 @@ class RequestException(LorisException):
     pass
 
 
-class ImageException(LorisException):
-    pass
-
-
 class ImageInfoException(LorisException):
     pass