Merge pull request #1552 from liip/simplify-proxy-resolver

cleanup proxy resolver

CHANGELOG.md

@@ -7,6 +7,7 @@ for a given releases. Unreleased, upcoming changes will be updated here periodic
 ## 3.x (unreleased)
 
 - The response when the `filter` parameter in a resolve request is not an array is now 400 bad request, and no longer 404 not found.
+- Simplified the `ProxyResolver` to no longer do the undocumented regex replace logic on domain names (reverting [#687](https://github.com/liip/LiipImagineBundle/pull/687)).
 
 # 2.x
 

doc/cache-resolver/proxy.rst

@@ -5,19 +5,19 @@ ProxyResolver
 =============
 
 The ``ProxyResolver`` cannot be used by itself. Instead, it is a "decorator" for
-another resolver. It add the ability to use "Proxy Hosts" for your assets. If no
-"Proxy Domains" are set, it behaves like the  underlying cache resolver.
+another resolver. It adds the ability to use "Proxy Hosts" for your assets. If no
+"Proxy Domains" are set, it behaves like the underlying cache resolver.
 
 Prerequisites
 -------------
 
 Create Service
 ~~~~~~~~~~~~~~
 
-To use this cache resolver, you must first define the cache resolver it will decorate.
+To use this cache resolver, you need to first define the cache resolver it will decorate.
 In this example, we will use the :ref:`AWS Cache Resolver <cache-resolver-aws-s3>`.
 
-Next, we need to define a service for this cache resolver and inject an array of domains
+We define a service for this cache resolver and inject an array of domains
 and the cache resolver service to decorate.
 
 .. code-block:: yaml
@@ -39,6 +39,9 @@ With this configuration, the cache resolver will generate paths such as
 returned from the decorated cache resolver, in this example using AWS,
 ``//bucket.s3.awsamazoncloud.com/.../image.jpg``).
 
+The domains you configure need the scheme (``http`` or ``https``), the full domain name and the port if it is not the
+default port for the scheme.
+
 Usage
 -----
 

src/Imagine/Cache/Resolver/ProxyResolver.php

@@ -56,29 +56,21 @@ public function remove(array $paths, array $filters): void
         $this->resolver->remove($paths, $filters);
     }
 
+    /**
+     * If you need more complex logic, extend the proxy resolver and overwrite this method.
+     */
     protected function rewriteUrl(string $url): string
     {
         if (empty($this->hosts)) {
             return $url;
         }
 
-        $randKey = array_rand($this->hosts, 1);
+        $randKey = array_rand($this->hosts);
 
-        // BC
-        if (is_numeric($randKey)) {
-            $port = parse_url($url, PHP_URL_PORT);
-            $host = parse_url($url, PHP_URL_SCHEME).'://'.parse_url($url, PHP_URL_HOST).($port ? ':'.$port : '');
-            $proxyHost = $this->hosts[$randKey];
-
-            return str_replace($host, $proxyHost, $url);
-        }
-
-        if (0 === mb_strpos($randKey, 'regexp/')) {
-            $regExp = mb_substr($randKey, 6);
-
-            return preg_replace($regExp, $this->hosts[$randKey], $url);
-        }
+        $port = parse_url($url, PHP_URL_PORT);
+        $host = parse_url($url, PHP_URL_SCHEME).'://'.parse_url($url, PHP_URL_HOST).($port ? ':'.$port : '');
+        $proxyHost = $this->hosts[$randKey];
 
-        return str_replace($randKey, $this->hosts[$randKey], $url);
+        return str_replace($host, $proxyHost, $url);
     }
 }

tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php

@@ -233,38 +233,6 @@ public function testWrapResolverWithProxyAndCacheOnCreate(): void
         $this->assertSame('liip_imagine.cache.resolver.the_resolver_name.cached', (string) $resolverDefinition->getArgument(1));
     }
 
-    public function testWrapResolverWithProxyMatchReplaceStrategyOnCreate(): void
-    {
-        $container = new ContainerBuilder();
-
-        $resolver = new AwsS3ResolverFactory();
-
-        $resolver->create($container, 'the_resolver_name', [
-            'client_config' => [],
-            'bucket' => 'aBucket',
-            'acl' => 'aAcl',
-            'get_options' => [],
-            'put_options' => [],
-            'cache' => 'the_cache_service_id',
-            'proxies' => ['foo' => 'bar'],
-        ]);
-
-        $this->assertTrue($container->hasDefinition('liip_imagine.cache.resolver.the_resolver_name.proxied'));
-        $proxiedResolverDefinition = $container->getDefinition('liip_imagine.cache.resolver.the_resolver_name.proxied');
-        $this->assertInstanceOf(ChildDefinition::class, $proxiedResolverDefinition);
-        $this->assertSame('liip_imagine.cache.resolver.prototype.aws_s3', $proxiedResolverDefinition->getParent());
-
-        $this->assertTrue($container->hasDefinition('liip_imagine.cache.resolver.the_resolver_name.cached'));
-        $cachedResolverDefinition = $container->getDefinition('liip_imagine.cache.resolver.the_resolver_name.cached');
-        $this->assertInstanceOf(ChildDefinition::class, $cachedResolverDefinition);
-        $this->assertSame('liip_imagine.cache.resolver.prototype.proxy', $cachedResolverDefinition->getParent());
-
-        $this->assertInstanceOf(Reference::class, $cachedResolverDefinition->getArgument(0));
-        $this->assertSame('liip_imagine.cache.resolver.the_resolver_name.proxied', (string) $cachedResolverDefinition->getArgument(0));
-
-        $this->assertSame(['foo' => 'bar'], $cachedResolverDefinition->getArgument(1));
-    }
-
     public function testSetCachePrefixIfDefined(): void
     {
         $container = new ContainerBuilder();

tests/Imagine/Cache/Resolver/ProxyResolverTest.php

@@ -70,46 +70,6 @@ public function testProxyCallAndRewriteReturnedUrlEvenSchemesDiffersOnResolve():
         $this->assertSame('http://images.example.com/thumbs/foo/bar/bazz.png', $result);
     }
 
-    public function testProxyCallAndRewriteReturnedUrlWithMatchReplaceOnResolve(): void
-    {
-        $expectedPath = '/foo/bar/bazz.png';
-        $expectedFilter = 'test';
-
-        $this->primaryResolver
-            ->expects($this->once())
-            ->method('resolve')
-            ->with($expectedPath, $expectedFilter)
-            ->willReturn('https://s3-eu-west-1.amazonaws.com/s3-cache.example.com/thumbs/foo/bar/bazz.png');
-
-        $this->resolver = new ProxyResolver($this->primaryResolver, [
-            'https://s3-eu-west-1.amazonaws.com/s3-cache.example.com' => 'http://images.example.com',
-        ]);
-
-        $result = $this->resolver->resolve($expectedPath, $expectedFilter);
-
-        $this->assertSame('http://images.example.com/thumbs/foo/bar/bazz.png', $result);
-    }
-
-    public function testProxyCallAndRewriteReturnedUrlWithRegExpOnResolve(): void
-    {
-        $expectedPath = '/foo/bar/bazz.png';
-        $expectedFilter = 'test';
-
-        $this->primaryResolver
-            ->expects($this->once())
-            ->method('resolve')
-            ->with($expectedPath, $expectedFilter)
-            ->willReturn('http://foo.com/thumbs/foo/bar/bazz.png');
-
-        $this->resolver = new ProxyResolver($this->primaryResolver, [
-            'regexp/http:\/\/.*?\//' => 'http://bar.com/',
-        ]);
-
-        $result = $this->resolver->resolve($expectedPath, $expectedFilter);
-
-        $this->assertSame('http://bar.com/thumbs/foo/bar/bazz.png', $result);
-    }
-
     public function testProxyCallAndReturnedValueOnIsStored(): void
     {
         $expectedPath = 'thePath';