From a9358db5b530d4819751da53535a16f2e2d20beb Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 21 Oct 2011 10:44:10 +0200 Subject: [PATCH 1/5] delegate response creation into the file manager and cache path resolver --- Controller/ImagineController.php | 17 ++++------- Imagine/CachePathResolver.php | 28 ++++++++++++++----- Imagine/Filter/FilterManager.php | 48 ++++++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 28 deletions(-) diff --git a/Controller/ImagineController.php b/Controller/ImagineController.php index 12b35a876..ecbeda2dd 100644 --- a/Controller/ImagineController.php +++ b/Controller/ImagineController.php @@ -69,21 +69,14 @@ public function filterAction(Request $request, $path, $filter) } $image = $this->dataLoader->find($path); - $targetFormat = pathinfo($path, PATHINFO_EXTENSION); - $image = $this->filterManager->get($filter, $image, $targetFormat); + $response = $this->filterManager->get($request, $filter, $image, $path); if ($targetPath) { - $this->cachePathResolver->store($targetPath, $image); - $statusCode = 201; - } else { - $statusCode = 200; + $this->cachePathResolver->store($targetPath, $response->getContent()); + $path = str_replace($this->webRoot, '', $targetPath); + $response = $this->cachePathResolver->redirect($request, $path); } - $contentType = $request->getMimeType($targetFormat); - if (empty($contentType)) { - $contentType = 'image/'.$targetFormat; - } - - return new Response($image, $statusCode, array('Content-Type' => $contentType)); + return $response; } } diff --git a/Imagine/CachePathResolver.php b/Imagine/CachePathResolver.php index c155489d3..c3c7cef95 100644 --- a/Imagine/CachePathResolver.php +++ b/Imagine/CachePathResolver.php @@ -3,7 +3,7 @@ namespace Liip\ImagineBundle\Imagine; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\Routing\RouterInterface; use Symfony\Component\HttpKernel\Util\Filesystem; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -28,9 +28,9 @@ class CachePathResolver /** * Constructs cache path resolver with a given web root and cache prefix * - * @param Symfony\Component\HttpFoundation\Request $request - * @param Symfony\Component\Routing\RouterInterface $router - * @param Symfony\Component\HttpKernel\Util\Filesystem $filesystem + * @param Request $request + * @param RouterInterface $router + * @param Filesystem $filesystem * @param string $webRoot */ public function __construct(RouterInterface $router, Filesystem $filesystem, $webRoot) @@ -91,14 +91,28 @@ public function resolve(Request $request, $targetPath, $filter) // if the file has already been cached, we're probably not rewriting // correctly, hence make a 301 to proper location, so browser remembers if (file_exists($targetPath)) { - return new Response('', 301, array( - 'location' => $request->getBasePath().$browserPath - )); + return $this->redirect($request, $browserPath); } return $targetPath; } + /** + * @param Request $request + * @param string $location + * @return RedirectResponse + */ + public function redirect(Request $request, $location) + { + return new RedirectResponse($request->getBasePath().$location); + } + + /** + * @throws \RuntimeException + * @param string $targetPath + * @param string $image + * @return void + */ public function store($targetPath, $image) { $dir = pathinfo($targetPath, PATHINFO_DIRNAME); diff --git a/Imagine/Filter/FilterManager.php b/Imagine/Filter/FilterManager.php index 111ad7704..55b4eff13 100644 --- a/Imagine/Filter/FilterManager.php +++ b/Imagine/Filter/FilterManager.php @@ -2,28 +2,39 @@ namespace Liip\ImagineBundle\Imagine\Filter; -use Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface; +use Symfony\Component\HttpFoundation\Request, + Symfony\Component\HttpFoundation\Response; -use Imagine\Exception\InvalidArgumentException; +use Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface; class FilterManager { private $filters; private $loaders; - private $services; + /** + * @param array $filters + */ public function __construct(array $filters = array()) { $this->filters = $filters; $this->loaders = array(); - $this->services = array(); } + /** + * @param $name + * @param Loader\LoaderInterface $loader + * @return void + */ public function addLoader($name, LoaderInterface $loader) { $this->loaders[$name] = $loader; } + /** + * @param $filter + * @return array + */ public function getFilterConfig($filter) { if (empty($this->filters[$filter])) { @@ -33,10 +44,17 @@ public function getFilterConfig($filter) return $this->filters[$filter]; } - public function get($filter, $image, $format = 'png') + /** + * @param Request $request + * @param $filter + * @param $image + * @param string $localPath + * @return Response + */ + public function get(Request $request, $filter, $image, $localPath) { if (!isset($this->filters[$filter])) { - throw new InvalidArgumentException(sprintf( + throw new \InvalidArgumentException(sprintf( 'Could not find image filter "%s"', $filter )); } @@ -45,17 +63,29 @@ public function get($filter, $image, $format = 'png') foreach ($config['filters'] as $filter => $options) { if (!isset($this->loaders[$filter])) { - throw new InvalidArgumentException(sprintf( + throw new \InvalidArgumentException(sprintf( 'Could not find loader for "%s" filter type', $filter )); } $image = $this->loaders[$filter]->load($image, $options); } + if (empty($config['format'])) { + $format = pathinfo($localPath, PATHINFO_EXTENSION); + $format = $format ?: 'png'; + } else { + $format = $config['format']; + } + $quality = empty($config['quality']) ? 100 : $config['quality']; - $format = empty($config['format']) ? $format : $config['format']; + $image = $image->get($format, array('quality' => $quality)); - return $image; + $contentType = $request->getMimeType($format); + if (empty($contentType)) { + $contentType = 'image/'.$format; + } + + return new Response($image, 200, array('Content-Type' => $contentType)); } } From e6a3b2928ad58bd9e83b5de8be2da445116b2f20 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 21 Oct 2011 10:50:30 +0200 Subject: [PATCH 2/5] do not redirect when caching --- Controller/ImagineController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Controller/ImagineController.php b/Controller/ImagineController.php index ecbeda2dd..e18da68b9 100644 --- a/Controller/ImagineController.php +++ b/Controller/ImagineController.php @@ -73,8 +73,7 @@ public function filterAction(Request $request, $path, $filter) if ($targetPath) { $this->cachePathResolver->store($targetPath, $response->getContent()); - $path = str_replace($this->webRoot, '', $targetPath); - $response = $this->cachePathResolver->redirect($request, $path); + $response->setStatusCode(201); } return $response; From 7ec7d32d3a77ffba6d33a79464d1b16820e87176 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 21 Oct 2011 11:05:25 +0200 Subject: [PATCH 3/5] removed no longer needed public method for redirecting --- Imagine/CachePathResolver.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/Imagine/CachePathResolver.php b/Imagine/CachePathResolver.php index c3c7cef95..b6ea0f42b 100644 --- a/Imagine/CachePathResolver.php +++ b/Imagine/CachePathResolver.php @@ -91,22 +91,12 @@ public function resolve(Request $request, $targetPath, $filter) // if the file has already been cached, we're probably not rewriting // correctly, hence make a 301 to proper location, so browser remembers if (file_exists($targetPath)) { - return $this->redirect($request, $browserPath); + return new RedirectResponse($request->getBasePath().$targetPath); } return $targetPath; } - /** - * @param Request $request - * @param string $location - * @return RedirectResponse - */ - public function redirect(Request $request, $location) - { - return new RedirectResponse($request->getBasePath().$location); - } - /** * @throws \RuntimeException * @param string $targetPath From d19838b0b9b863e5f90b3c6696cea4768c18b33c Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 21 Oct 2011 15:30:10 +0200 Subject: [PATCH 4/5] make cache path resolver store() method return a response --- Controller/ImagineController.php | 3 +-- Imagine/CachePathResolver.php | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Controller/ImagineController.php b/Controller/ImagineController.php index e18da68b9..0ec5300da 100644 --- a/Controller/ImagineController.php +++ b/Controller/ImagineController.php @@ -72,8 +72,7 @@ public function filterAction(Request $request, $path, $filter) $response = $this->filterManager->get($request, $filter, $image, $path); if ($targetPath) { - $this->cachePathResolver->store($targetPath, $response->getContent()); - $response->setStatusCode(201); + $response = $this->cachePathResolver->store($response, $targetPath); } return $response; diff --git a/Imagine/CachePathResolver.php b/Imagine/CachePathResolver.php index b6ea0f42b..ecf5d4c2f 100644 --- a/Imagine/CachePathResolver.php +++ b/Imagine/CachePathResolver.php @@ -2,11 +2,12 @@ namespace Liip\ImagineBundle\Imagine; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RedirectResponse; -use Symfony\Component\Routing\RouterInterface; -use Symfony\Component\HttpKernel\Util\Filesystem; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\HttpFoundation\Request, + Symfony\Component\HttpFoundation\Response, + Symfony\Component\HttpFoundation\RedirectResponse, + Symfony\Component\Routing\RouterInterface, + Symfony\Component\HttpKernel\Util\Filesystem, + Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class CachePathResolver { @@ -99,11 +100,11 @@ public function resolve(Request $request, $targetPath, $filter) /** * @throws \RuntimeException + * @param Response $response * @param string $targetPath - * @param string $image - * @return void + * @return Response */ - public function store($targetPath, $image) + public function store(Response $response, $targetPath) { $dir = pathinfo($targetPath, PATHINFO_DIRNAME); @@ -113,6 +114,10 @@ public function store($targetPath, $image) )); } - file_put_contents($targetPath, $image); + file_put_contents($targetPath, $response->getContent()); + + $response->setStatusCode(201); + + return $response; } } From 53cd96a0dc67ebef906795c10f863b710e6e3f2d Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 21 Oct 2011 15:33:24 +0200 Subject: [PATCH 5/5] WS tweaks --- Controller/ImagineController.php | 13 +++++++------ Imagine/CachePathResolver.php | 15 ++++++++++----- Imagine/Filter/FilterManager.php | 10 ++++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/Controller/ImagineController.php b/Controller/ImagineController.php index 0ec5300da..5345127c2 100644 --- a/Controller/ImagineController.php +++ b/Controller/ImagineController.php @@ -11,12 +11,12 @@ class ImagineController { /** - * @var Liip\ImagineBundle\Imagine\DataLoader\LoaderInterface + * @var LoaderInterface */ private $dataLoader; /** - * @var Liip\ImagineBundle\Imagine\Filter\FilterManager + * @var FilterManager */ private $filterManager; @@ -26,16 +26,17 @@ class ImagineController private $webRoot; /** - * @var Liip\ImagineBundle\Imagine\CachePathResolver + * @var CachePathResolver */ private $cachePathResolver; /** * Constructor * - * @param Liip\ImagineBundle\Imagine\DataLoader\LoaderInterface $dataLoader - * @param Liip\ImagineBundle\Imagine\Filter\FilterManager $filterManager - * @param Liip\ImagineBundle\Imagine\CachePathResolver $cachePathResolver + * @param LoaderInterface $dataLoader + * @param FilterManager $filterManager + * @param string $webRoot + * @param CachePathResolver $cachePathResolver */ public function __construct(LoaderInterface $dataLoader, FilterManager $filterManager, $webRoot, CachePathResolver $cachePathResolver = null) { diff --git a/Imagine/CachePathResolver.php b/Imagine/CachePathResolver.php index ecf5d4c2f..7bf6a6b63 100644 --- a/Imagine/CachePathResolver.php +++ b/Imagine/CachePathResolver.php @@ -12,12 +12,12 @@ class CachePathResolver { /** - * @var Symfony\Component\Routing\RouterInterface + * @var RouterInterface */ private $router; /** - * @var Symfony\Component\HttpKernel\Util\Filesystem + * @var Filesystem */ private $filesystem; @@ -29,10 +29,10 @@ class CachePathResolver /** * Constructs cache path resolver with a given web root and cache prefix * - * @param Request $request - * @param RouterInterface $router + * @param Request $request + * @param RouterInterface $router * @param Filesystem $filesystem - * @param string $webRoot + * @param string $webRoot */ public function __construct(RouterInterface $router, Filesystem $filesystem, $webRoot) { @@ -47,6 +47,8 @@ public function __construct(RouterInterface $router, Filesystem $filesystem, $we * @param string $path * @param string $filter * @param boolean $absolute + * + * @return string */ public function getBrowserPath($targetPath, $filter, $absolute = false) { @@ -71,6 +73,8 @@ public function getBrowserPath($targetPath, $filter, $absolute = false) * @param Request $request * @param string $path * @param string $filter + * + * @return string */ public function resolve(Request $request, $targetPath, $filter) { @@ -102,6 +106,7 @@ public function resolve(Request $request, $targetPath, $filter) * @throws \RuntimeException * @param Response $response * @param string $targetPath + * * @return Response */ public function store(Response $response, $targetPath) diff --git a/Imagine/Filter/FilterManager.php b/Imagine/Filter/FilterManager.php index 55b4eff13..3e050f4e9 100644 --- a/Imagine/Filter/FilterManager.php +++ b/Imagine/Filter/FilterManager.php @@ -9,7 +9,14 @@ class FilterManager { + /** + * @var array + */ private $filters; + + /** + * @var array + */ private $loaders; /** @@ -24,6 +31,7 @@ public function __construct(array $filters = array()) /** * @param $name * @param Loader\LoaderInterface $loader + * * @return void */ public function addLoader($name, LoaderInterface $loader) @@ -33,6 +41,7 @@ public function addLoader($name, LoaderInterface $loader) /** * @param $filter + * * @return array */ public function getFilterConfig($filter) @@ -49,6 +58,7 @@ public function getFilterConfig($filter) * @param $filter * @param $image * @param string $localPath + * * @return Response */ public function get(Request $request, $filter, $image, $localPath)