diff --git a/CHANGELOG.md b/CHANGELOG.md index ca9814f07..325876876 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/cache-resolver/proxy.rst b/doc/cache-resolver/proxy.rst index 4b70efac2..21034395e 100644 --- a/doc/cache-resolver/proxy.rst +++ b/doc/cache-resolver/proxy.rst @@ -5,8 +5,8 @@ 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 ------------- @@ -14,10 +14,10 @@ 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 `. -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 ----- diff --git a/src/Imagine/Cache/Resolver/ProxyResolver.php b/src/Imagine/Cache/Resolver/ProxyResolver.php index 9ddfab048..59069dd41 100644 --- a/src/Imagine/Cache/Resolver/ProxyResolver.php +++ b/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); } } diff --git a/tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php b/tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php index 10d4677ab..a83b7e35f 100644 --- a/tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php +++ b/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(); diff --git a/tests/Imagine/Cache/Resolver/ProxyResolverTest.php b/tests/Imagine/Cache/Resolver/ProxyResolverTest.php index c88484db7..1698f5880 100644 --- a/tests/Imagine/Cache/Resolver/ProxyResolverTest.php +++ b/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';