diff --git a/loris/img.py b/loris/img.py index 2470173d..5c08bcb9 100644 --- a/loris/img.py +++ b/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 diff --git a/loris/loris_exception.py b/loris/loris_exception.py index ec35cb1f..2b94989d 100644 --- a/loris/loris_exception.py +++ b/loris/loris_exception.py @@ -13,10 +13,6 @@ class RequestException(LorisException): pass -class ImageException(LorisException): - pass - - class ImageInfoException(LorisException): pass diff --git a/loris/parameters.py b/loris/parameters.py index b5d96b85..e32497b6 100644 --- a/loris/parameters.py +++ b/loris/parameters.py @@ -35,7 +35,7 @@ class RegionParameter(object): The normalized (pixel-based, in-bounds) region slice of the URI. mode (str): One of 'full', 'square', 'pct', or 'pixel' - img_info (ImageInfo) + image_info (ImageInfo) pixel_x (int) decimal_x (Decimal) pixel_y (int) @@ -47,26 +47,26 @@ class RegionParameter(object): ''' __slots__ = ('uri_value','canonical_uri_value','pixel_x','decimal_x', 'pixel_y','decimal_y','pixel_w','decimal_w','pixel_h','decimal_h', - 'mode','img_info') + 'mode','image_info') def __str__(self): return self.uri_value - def __init__(self, uri_value, img_info): + def __init__(self, uri_value, image_info): '''Parse the uri_value into the object. Args: uri_value (str): The region slice of an IIIF image request URI. - img_info (ImgInfo) + image_info (ImgInfo) Raises: SyntaxException RequestException ''' self.uri_value = uri_value - self.img_info = img_info + self.image_info = image_info - self.mode = RegionParameter._mode_from_region_segment(self.uri_value, self.img_info) + self.mode = RegionParameter._mode_from_region_segment(self.uri_value, self.image_info) logger.debug('Region mode is "%s" (from "%s")', self.mode, uri_value) @@ -106,12 +106,12 @@ def _canonicalize(self): def _adjust_to_in_bounds(self): if (self.decimal_x + self.decimal_w) > DECIMAL_ONE: self.decimal_w = DECIMAL_ONE - self.decimal_x - self.pixel_w = self.img_info.width - self.pixel_x + self.pixel_w = self.image_info.width - self.pixel_x logger.info('decimal_w adjusted to: %s', self.decimal_w) logger.info('pixel_w adjusted to: %d', self.pixel_w) if (self.decimal_y + self.decimal_h) > DECIMAL_ONE: self.decimal_h = DECIMAL_ONE - self.decimal_y - self.pixel_h = self.img_info.height - self.pixel_y + self.pixel_h = self.image_info.height - self.pixel_y logger.info('decimal_h adjusted to: %s', self.decimal_h) logger.debug('pixel_h adjusted to: %s', self.pixel_h) @@ -124,12 +124,12 @@ def _check_for_oob_errors(self): if self.decimal_x >= DECIMAL_ONE: raise RequestException( "Region x parameter is greater than the width of the image. " - "Image width is %d" % self.img_info.width + "Image width is %d" % self.image_info.width ) if self.decimal_y >= DECIMAL_ONE: raise RequestException( "Region y parameter is greater than the height of the image. " - "Image height is %d" % self.img_info.height + "Image height is %d" % self.image_info.height ) def _populate_slots_for_full(self): @@ -138,9 +138,9 @@ def _populate_slots_for_full(self): self.decimal_x = 0 self.pixel_y = 0 self.decimal_y = 0 - self.pixel_w = self.img_info.width + self.pixel_w = self.image_info.width self.decimal_w = DECIMAL_ONE - self.pixel_h = self.img_info.height + self.pixel_h = self.image_info.height self.decimal_h = DECIMAL_ONE def _populate_slots_from_pct(self): @@ -168,10 +168,10 @@ def _populate_slots_from_pct(self): self.decimal_h = [RegionParameter._pct_to_decimal(d) for d in dimensions] # pixels - self.pixel_x = int(floor(self.decimal_x * self.img_info.width)) - self.pixel_y = int(floor(self.decimal_y * self.img_info.height)) - self.pixel_w = int(floor(self.decimal_w * self.img_info.width)) - self.pixel_h = int(floor(self.decimal_h * self.img_info.height)) + self.pixel_x = int(floor(self.decimal_x * self.image_info.width)) + self.pixel_y = int(floor(self.decimal_y * self.image_info.height)) + self.pixel_w = int(floor(self.decimal_w * self.image_info.width)) + self.pixel_h = int(floor(self.decimal_h * self.image_info.height)) def _populate_slots_for_square(self): ''' @@ -180,12 +180,12 @@ def _populate_slots_for_square(self): SyntaxException ''' #dimensions must be ints, for passing to _populate_slots_from_pixels - if self.img_info.width > self.img_info.height: - offset = (self.img_info.width - self.img_info.height) // 2 - dimensions = (offset, 0, self.img_info.height, self.img_info.height) + if self.image_info.width > self.image_info.height: + offset = (self.image_info.width - self.image_info.height) // 2 + dimensions = (offset, 0, self.image_info.height, self.image_info.height) else: - offset = (self.img_info.height - self.img_info.width) // 2 - dimensions = (0, offset, self.img_info.width, self.img_info.width) + offset = (self.image_info.height - self.image_info.width) // 2 + dimensions = (0, offset, self.image_info.width, self.image_info.width) return self._populate_slots_from_pixels(dimensions) def _pixel_dims_to_ints(self): @@ -200,13 +200,13 @@ def _populate_slots_from_pixels(self, dimensions): # pixels self.pixel_x, self.pixel_y, self.pixel_w, self.pixel_h = dimensions # decimals - self.decimal_x = Decimal(self.pixel_x) / Decimal(str(self.img_info.width)) - self.decimal_y = Decimal(self.pixel_y) / Decimal(str(self.img_info.height)) - self.decimal_w = Decimal(self.pixel_w) / Decimal(str(self.img_info.width)) - self.decimal_h = Decimal(self.pixel_h) / Decimal(str(self.img_info.height)) + self.decimal_x = Decimal(self.pixel_x) / Decimal(str(self.image_info.width)) + self.decimal_y = Decimal(self.pixel_y) / Decimal(str(self.image_info.height)) + self.decimal_w = Decimal(self.pixel_w) / Decimal(str(self.image_info.width)) + self.decimal_h = Decimal(self.pixel_h) / Decimal(str(self.image_info.height)) @staticmethod - def _mode_from_region_segment(region_segment, img_info): + def _mode_from_region_segment(region_segment, image_info): ''' Get the mode of the request from the region segment. @@ -227,8 +227,8 @@ def _mode_from_region_segment(region_segment, img_info): if len(comma_segments) == 4 and all([ comma_segments[0] == '0', comma_segments[1] == '0', - comma_segments[2] == str(img_info.width), - comma_segments[3] == str(img_info.height) + comma_segments[2] == str(image_info.width), + comma_segments[3] == str(image_info.height) ]): return FULL_MODE elif all([n.isdigit() for n in comma_segments]): diff --git a/loris/transforms.py b/loris/transforms.py index 6aa0993f..7c828c31 100644 --- a/loris/transforms.py +++ b/loris/transforms.py @@ -65,12 +65,12 @@ def __init__(self, config): self.dither_bitonal_images = config['dither_bitonal_images'] logger.debug('Initialized %s.%s', __name__, self.__class__.__name__) - def transform(self, src_fp, target_fp, image_request): + def transform(self, target_fp, image_request, image_info): ''' Args: - src_fp (str) target_fp (str) - image (ImageRequest) + image_request (ImageRequest) + image_info (ImageInfo) ''' cn = self.__class__.__name__ raise NotImplementedError('transform() not implemented for %s' % (cn,)) @@ -86,7 +86,7 @@ def srgb_profile_fp(self): def _map_im_profile_to_srgb(self, im, input_profile): return profileToProfile(im, input_profile, self.srgb_profile_fp) - def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True): + def _derive_with_pil(self, im, target_fp, image_request, image_info, rotate=True, crop=True): ''' Once you have a PIL.Image, this can be used to do the IIIF operations. @@ -94,6 +94,7 @@ def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True) im (PIL.Image) target_fp (str) image_request (ImageRequest) + image_info (ImageInfo) rotate (bool): True by default; can be set to False in case the rotation was done further upstream. @@ -104,27 +105,31 @@ def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True) void (puts an image at target_fp) ''' + region_param = image_request.region_param(image_info=image_info) - if crop and image_request.region_param.canonical_uri_value != 'full': + if crop and region_param.canonical_uri_value != 'full': # For PIL: "The box is a 4-tuple defining the left, upper, right, # and lower pixel coordinate." box = ( - image_request.region_param.pixel_x, - image_request.region_param.pixel_y, - image_request.region_param.pixel_x+image_request.region_param.pixel_w, - image_request.region_param.pixel_y+image_request.region_param.pixel_h + region_param.pixel_x, + region_param.pixel_y, + region_param.pixel_x + region_param.pixel_w, + region_param.pixel_y + region_param.pixel_h ) logger.debug('cropping to: %r', box) im = im.crop(box) # resize - if image_request.size_param.canonical_uri_value != 'full': - wh = [int(image_request.size_param.w),int(image_request.size_param.h)] + size_param = image_request.size_param(image_info=image_info) + + if size_param.canonical_uri_value != 'full': + wh = [int(size_param.w), int(size_param.h)] logger.debug('Resizing to: %r', wh) im = im.resize(wh, resample=Image.ANTIALIAS) + rotation_param = image_request.rotation_param() - if image_request.rotation_param.mirror: + if rotation_param.mirror: im = mirror(im) try: @@ -134,14 +139,15 @@ def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True) except PyCMSError as err: logger.warn('Error converting %r to sRGB: %r', im, err) - if image_request.rotation_param.rotation != '0' and rotate: - r = 0-float(image_request.rotation_param.rotation) + if rotation_param.rotation != '0' and rotate: + r = 0 - float(rotation_param.rotation) # We need to convert pngs here and not below if we want a # transparent background (A == Alpha layer) - if float(image_request.rotation_param.rotation) % 90 != 0.0 and \ - image_request.format == 'png': - + if ( + float(rotation_param.rotation) % 90 != 0.0 and + image_request.format == 'png' + ): if image_request.quality in ('gray', 'bitonal'): im = im.convert('LA') else: @@ -150,7 +156,10 @@ def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True) im = im.rotate(r, expand=True) if not im.mode.endswith('A'): - if im.mode != "RGB" and not image_request.quality in ('gray', 'bitonal'): + if ( + im.mode != "RGB" and + image_request.quality not in ('gray', 'bitonal') + ): im = im.convert("RGB") elif image_request.quality == 'gray': @@ -179,9 +188,14 @@ def _derive_with_pil(self, im, target_fp, image_request, rotate=True, crop=True) class _PillowTransformer(_AbstractTransformer): - def transform(self, src_fp, target_fp, image_request): - im = Image.open(src_fp) - self._derive_with_pil(im, target_fp, image_request) + def transform(self, target_fp, image_request, image_info): + im = Image.open(image_info.src_img_fp) + self._derive_with_pil( + im=im, + target_fp=target_fp, + image_request=image_request, + image_info=image_info + ) class JPG_Transformer(_PillowTransformer): @@ -233,17 +247,17 @@ def _get_closest_scale(self, req_w, req_h, full_w, full_h, scales): if self._scale_dim(full_w,s) >= req_w and \ self._scale_dim(full_h,s) >= req_h]) - def _scales_to_reduce_arg(self, image_request): + def _scales_to_reduce_arg(self, image_request, image_info): # Scales from JP2 levels, so even though these are from the tiles # info.json, it's easier than using the sizes from info.json - scales = [s for t in image_request.info.tiles for s in t['scaleFactors']] - is_full_region = image_request.region_param.mode == FULL_MODE + scales = [s for t in image_info.tiles for s in t['scaleFactors']] + is_full_region = image_request.region_param(image_info).mode == FULL_MODE arg = None if scales and is_full_region: - full_w = image_request.info.width - full_h = image_request.info.height - req_w = image_request.size_param.w - req_h = image_request.size_param.h + full_w = image_info.width + full_h = image_info.height + req_w = image_request.size_param(image_info).w + req_h = image_request.size_param(image_info).h closest_scale = self._get_closest_scale(req_w, req_h, full_w, full_h, scales) reduce_arg = int(log(closest_scale, 2)) arg = str(reduce_arg) @@ -287,7 +301,7 @@ def _region_to_opj_arg(self, region_param): logger.debug('opj region parameter: %s', arg) return arg - def transform(self, src_fp, target_fp, image_request): + def transform(self, target_fp, image_request, image_info): # opj writes to this: fifo_fp = self._make_tmp_fp() @@ -300,11 +314,11 @@ def transform(self, src_fp, target_fp, image_request): # how to handle CalledProcessError; would have to be a 500? # opj_decompress command - i = '-i "%s"' % (src_fp,) + i = '-i "%s"' % (image_info.src_img_fp,) o = '-o %s' % (fifo_fp,) - region_arg = self._region_to_opj_arg(image_request.region_param) + region_arg = self._region_to_opj_arg(image_request.region_param(image_info)) reg = '-d %s' % (region_arg,) if region_arg else '' - reduce_arg = self._scales_to_reduce_arg(image_request) + reduce_arg = self._scales_to_reduce_arg(image_request, image_info) red = '-r %s' % (reduce_arg,) if reduce_arg else '' opj_cmd = ' '.join((self.opj_decompress,i,reg,red,o)) @@ -335,13 +349,19 @@ def transform(self, src_fp, target_fp, image_request): unlink(fifo_fp) try: - if self.map_profile_to_srgb and image_request.info.color_profile_bytes: # i.e. is not None - emb_profile = BytesIO(image_request.info.color_profile_bytes) + if self.map_profile_to_srgb and image_info.color_profile_bytes: + emb_profile = BytesIO(image_info.color_profile_bytes) im = self._map_im_profile_to_srgb(im, emb_profile) except PyCMSError as err: logger.warn('Error converting %r to sRGB: %r', im, err) - self._derive_with_pil(im, target_fp, image_request, crop=False) + self._derive_with_pil( + im=im, + target_fp=target_fp, + image_request=image_request, + image_info=image_info, + crop=False + ) class KakaduJP2Transformer(_AbstractJP2Transformer): @@ -385,7 +405,7 @@ def _region_to_kdu_arg(self, region_param): logger.debug('kdu region parameter: %s', arg) return arg - def _run_transform(self, target_fp, image_request, kdu_cmd, fifo_fp): + def _run_transform(self, target_fp, image_request, image_info, kdu_cmd, fifo_fp): try: # Start the kdu shellout. Blocks until the pipe is empty kdu_expand_proc = subprocess.Popen(kdu_cmd, shell=True, bufsize=-1, @@ -407,15 +427,21 @@ def _run_transform(self, target_fp, image_request, kdu_cmd, fifo_fp): unlink(fifo_fp) try: - if self.map_profile_to_srgb and image_request.info.color_profile_bytes: # i.e. is not None - emb_profile = BytesIO(image_request.info.color_profile_bytes) + if self.map_profile_to_srgb and image_info.color_profile_bytes: + emb_profile = BytesIO(image_info.color_profile_bytes) im = self._map_im_profile_to_srgb(im, emb_profile) except PyCMSError as err: logger.warn('Error converting %r to sRGB: %r', im, err) - self._derive_with_pil(im, target_fp, image_request, crop=False) + self._derive_with_pil( + im=im, + target_fp=target_fp, + image_request=image_request, + image_info=image_info, + crop=False + ) - def transform(self, src_fp, target_fp, image_request): + def transform(self, target_fp, image_request, image_info): fifo_fp = self._make_tmp_fp() mkfifo_call = '%s %s' % (self.mkfifo, fifo_fp) subprocess.check_call(mkfifo_call, shell=True) @@ -423,20 +449,29 @@ def transform(self, src_fp, target_fp, image_request): # kdu command q = '-quiet' t = '-num_threads %s' % self.num_threads - i = '-i "%s"' % src_fp + i = '-i "%s"' % image_info.src_img_fp o = '-o %s' % fifo_fp - reduce_arg = self._scales_to_reduce_arg(image_request) + reduce_arg = self._scales_to_reduce_arg(image_request, image_info) red = '-reduce %s' % (reduce_arg,) if reduce_arg else '' - region_arg = self._region_to_kdu_arg(image_request.region_param) + region_arg = self._region_to_kdu_arg(image_request.region_param(image_info)) reg = '-region %s' % (region_arg,) if region_arg else '' kdu_cmd = ' '.join((self.kdu_expand,q,i,t,reg,red,o)) - process = multiprocessing.Process(target=self._run_transform, - args=(target_fp, image_request, kdu_cmd, fifo_fp)) + process = multiprocessing.Process( + target=self._run_transform, + kwargs={ + 'target_fp': target_fp, + 'image_request': image_request, + 'image_info': image_info, + 'kdu_cmd': kdu_cmd, + 'fifo_fp': fifo_fp + } + ) process.start() process.join(self.transform_timeout) if process.is_alive(): - logger.info('terminating process for %s, %s', src_fp, target_fp) + logger.info('terminating process for %s, %s', + image_info.src_img_fp, target_fp) process.terminate() if path.exists(fifo_fp): unlink(fifo_fp) diff --git a/loris/webapp.py b/loris/webapp.py index 6e371a22..34cb458f 100755 --- a/loris/webapp.py +++ b/loris/webapp.py @@ -35,7 +35,6 @@ from loris.img_info import InfoCache from loris.loris_exception import ( ConfigError, - ImageException, ImageInfoException, RequestException, ResolverException, @@ -549,13 +548,19 @@ def _get_info(self,ident,request,base_uri): return (info,last_mod) - def _set_canonical_link(self, request, image_request, response): + def _set_canonical_link( + self, request, response, image_request, image_info + ): if self.proxy_path: root = self.proxy_path else: root = request.url_root - canonical_uri = '%s%s' % (root, image_request.canonical_request_path) - response.headers['Link'] = '%s,<%s>;rel="canonical"' % (response.headers['Link'], canonical_uri,) + + canonical_path = image_request.canonical_request_path(image_info) + canonical_uri = '%s%s' % (root, canonical_path) + response.headers['Link'] = '%s,<%s>;rel="canonical"' % ( + response.headers['Link'], canonical_uri + ) def get_img(self, request, ident, region, size, rotation, quality, target_fmt, base_uri): '''Get an Image. @@ -619,36 +624,43 @@ def get_img(self, request, ident, region, size, rotation, quality, target_fmt, b # hand the Image object its info info = self._get_info(ident, request, base_uri)[0] - image_request.info = info - # we need to do the above to set the canonical link header - self._set_canonical_link(request, image_request, r) + self._set_canonical_link( + request=request, + response=r, + image_request=image_request, + image_info=info + ) return r else: try: # 1. Get the info info = self._get_info(ident, request, base_uri)[0] - # 2. Give the image its info - image_request.info = info - # 3. Check that we can make the quality requested + # 2. Check that we can make the quality requested if image_request.quality not in info.profile.description['qualities']: return BadRequestResponse('"%s" quality is not available for this image' % (image_request.quality,)) - # 4. Check if requested size is allowed - if image_request.request_resolution_too_large(self.max_size_above_full): + # 3. Check if requested size is allowed + if image_request.request_resolution_too_large( + max_size_above_full=self.max_size_above_full, + image_info=info + ): return NotFoundResponse('Resolution not available') - # 5. Redirect if appropriate + # 4. Redirect if appropriate if self.redirect_canonical_image_request: - if not image_request.is_canonical: + if not image_request.is_canonical(info): self.logger.debug('Attempting redirect to %s', image_request.canonical_request_path,) r.headers['Location'] = image_request.canonical_request_path r.status_code = 301 return r - # 6. Make an image - fp = self._make_image(image_request, info.src_img_fp, info.src_format) + # 5. Make an image + fp = self._make_image( + image_request=image_request, + image_info=info + ) except ResolverException as re: return NotFoundResponse(str(re)) @@ -656,11 +668,8 @@ def get_img(self, request, ident, region, size, rotation, quality, target_fmt, b return ServerSideErrorResponse(te) except (RequestException, SyntaxException) as e: return BadRequestResponse(str(e)) - except (ImageException,ImageInfoException) as ie: + except ImageInfoException as ie: # 500s! - # ImageException is only raised in when ImageRequest.info - # isn't set and is a developer error. It should never happen! - # # ImageInfoException is only raised when # ImageInfo.from_image_file() can't determine the format of the # source image. It results in a 500, but isn't necessarily a @@ -678,7 +687,12 @@ def get_img(self, request, ident, region, size, rotation, quality, target_fmt, b r.status_code = 200 r.last_modified = datetime.utcfromtimestamp(path.getctime(fp)) r.headers['Content-Length'] = path.getsize(fp) - self._set_canonical_link(request, image_request, r) + self._set_canonical_link( + request=request, + response=r, + image_request=image_request, + image_info=info + ) r.response = open(fp, 'rb') if not self.enable_caching: @@ -686,25 +700,42 @@ def get_img(self, request, ident, region, size, rotation, quality, target_fmt, b return r - def _make_image(self, image_request, src_fp, src_format): - ''' + def _make_image(self, image_request, image_info): + """Call the appropriate transformer to create the image. + Args: - image_request (img.ImageRequest) - src_fp (str) - src_format (str) + image_request (ImageRequest) + image_info (ImageInfo) Returns: - (str) the fp of the new image - ''' - temp_file = NamedTemporaryFile(dir=self.tmp_dp, suffix='.%s' % image_request.format, delete=False) + (str) the file path of the new image + + """ + temp_file = NamedTemporaryFile( + dir=self.tmp_dp, + suffix='.%s' % image_request.format, + delete=False + ) temp_fp = temp_file.name - transformer = self.transformers[src_format] - transformer.transform(src_fp, temp_fp, image_request) + transformer = self.transformers[image_info.src_format] + transformer.transform( + target_fp=temp_fp, + image_request=image_request, + image_info=image_info + ) if self.enable_caching: - temp_fp = self.img_cache.upsert(image_request, temp_fp) + temp_fp = self.img_cache.upsert( + image_request=image_request, + temp_fp=temp_fp, + image_info=image_info + ) # TODO: not sure how the non-canonical use case works - self.img_cache[image_request] = temp_fp + self.img_cache.store( + image_request=image_request, + image_info=image_info, + canonical_fp=temp_fp + ) return temp_fp diff --git a/requirements.in b/requirements.in index 499afd4d..8aa51ebb 100644 --- a/requirements.in +++ b/requirements.in @@ -11,4 +11,6 @@ requests >= 2.12.0 netaddr >= 0.7.19 pyjwt >= 1.5.2 cryptography >= 2.0.3 -attrs >= 17.3.0 + +# We use `attr.ib(converter=callable)` which was added in 17.4.0. +attrs >= 17.4.0 diff --git a/tests/img_t.py b/tests/img_t.py index 32c5f1b5..f26290fc 100644 --- a/tests/img_t.py +++ b/tests/img_t.py @@ -15,17 +15,11 @@ from urllib import unquote from loris import img, img_info -from loris.loris_exception import ImageException from tests import loris_t class TestImageRequest(object): - def test_missing_info_attribute_is_error(self): - request = img.ImageRequest('id1', 'full', 'full', '0', 'default', 'jpg') - with pytest.raises(ImageException): - request.info - @pytest.mark.parametrize('args, request_path', [ (('id1', 'full', 'full', '0', 'default', 'jpg'), 'id1/full/full/0/default.jpg'), @@ -34,9 +28,6 @@ def test_missing_info_attribute_is_error(self): ]) def test_request_path(self, args, request_path): request = img.ImageRequest(*args) - - # Called twice for caching behaviour - assert request.request_path == request_path assert request.request_path == request_path @pytest.mark.parametrize('args, is_canonical', [ @@ -52,11 +43,8 @@ def test_is_canonical(self, args, is_canonical): info.width = 100 info.height = 100 request = img.ImageRequest(*args) - request.info = info - # Called twice for caching behaviour - assert request.is_canonical == is_canonical - assert request.is_canonical == is_canonical + assert request.is_canonical(info) == is_canonical class Test_ImageCache(loris_t.LorisTest): @@ -104,11 +92,10 @@ def test_cache_dir_already_exists(self): image_info.width = 100 image_info.height = 100 image_request = img.ImageRequest(ident, 'full', 'full', '0', 'default', 'jpg') - image_request.info = image_info - self.app.img_cache.create_dir_and_return_file_path(image_request) + self.app.img_cache.create_dir_and_return_file_path(image_request, image_info) #call request again, so cache directory should already be there # throws an exception if we don't handle that existence properly - self.app.img_cache.create_dir_and_return_file_path(image_request) + self.app.img_cache.create_dir_and_return_file_path(image_request, image_info) def test_missing_entry_is_keyerror(self): cache = img.ImageCache(cache_root='/tmp') diff --git a/tests/transforms_t.py b/tests/transforms_t.py index 557d578f..b23af1a4 100644 --- a/tests/transforms_t.py +++ b/tests/transforms_t.py @@ -80,7 +80,7 @@ def test_missing_transform_raises_not_implemented_error(self): 'dither_bitonal_images': '', }) with pytest.raises(NotImplementedError) as err: - e.transform(src_fp=None, target_fp=None, image_request=None) + e.transform(target_fp=None, image_request=None, image_info=None) assert str(err.value) == 'transform() not implemented for ExampleTransformer' @pytest.mark.parametrize('config', [