Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Filters] [Config] [DI] Add Filter configuration class as public service #1098

Merged
merged 31 commits into from Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6b41c3b
added FilterSetCollection public service
maximgubar May 23, 2018
11c0c12
fixed static tests
maximgubar May 23, 2018
6a8cff3
removed setters from Filters and FilterSets
maximgubar May 24, 2018
add533f
fixed static tests
maximgubar May 24, 2018
95db82b
added filter-specific objects
maximgubar May 29, 2018
d907670
fixed static tests
maximgubar May 29, 2018
e213124
applied requested review changes
maximgubar Jun 4, 2018
851a38b
fixed static tests
maximgubar Jun 4, 2018
27c8794
removed FilterConfiguration object dependency
maximgubar Jun 4, 2018
5c4d68f
renamed FilterSet to Stack
maximgubar Jun 4, 2018
e306612
added filters config objects
maximgubar Jun 4, 2018
db0a518
added filters config objects
maximgubar Jun 6, 2018
d6c599f
fixed static tests
maximgubar Jun 6, 2018
6906000
added optional return types, removed unnecessary phpdocs
maximgubar Jun 6, 2018
4ea4acc
added Point and Size objects
maximgubar Jun 6, 2018
ebc5744
replaced trivial ternary operations with null coalesce operator
maximgubar Jun 6, 2018
02e4022
applied PR fixes
maximgubar Jun 6, 2018
c4923d0
added @internal to all config-related factories
maximgubar Jun 11, 2018
c564a62
introduced StckFactoryInterface
maximgubar Jun 11, 2018
9971029
moved filter NAME const from factory to instance
maximgubar Jun 11, 2018
36d7843
fixed static tests
maximgubar Jun 11, 2018
91bfe94
applied PR fixes
maximgubar Jun 11, 2018
9df4d4b
added @codeCoverageIgnore
maximgubar Jun 11, 2018
be890e3
applied PR fixes
maximgubar Jun 11, 2018
8458b37
renamed filterSet to stack
maximgubar Jun 12, 2018
6613b61
removed self-referencing constant
maximgubar Jun 12, 2018
cdab3b8
fixed static tests
maximgubar Jun 13, 2018
f8cdfb7
fixed small issues:
maximgubar Jun 13, 2018
2a397ae
fixed Watermark factory arguments order and type sanitize
maximgubar Jun 18, 2018
f1ed641
Merge branch '2.0' into config-reader
maximgubar Jun 25, 2018
0624a2a
removed backslashes from native constants
maximgubar Jun 25, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions Binary/Locator/FileSystemInsecureLocator.php
Expand Up @@ -21,9 +21,9 @@ class FileSystemInsecureLocator extends FileSystemLocator
*/
protected function generateAbsolutePath(string $root, string $path): ?string
{
if (false === mb_strpos($path, '..'.DIRECTORY_SEPARATOR) &&
false === mb_strpos($path, DIRECTORY_SEPARATOR.'..') &&
false !== file_exists($absolute = $root.DIRECTORY_SEPARATOR.$path)
if (false === mb_strpos($path, '..'.\DIRECTORY_SEPARATOR) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the backslashes here?

false === mb_strpos($path, \DIRECTORY_SEPARATOR.'..') &&
false !== file_exists($absolute = $root.\DIRECTORY_SEPARATOR.$path)
) {
return $absolute;
}
Expand Down
2 changes: 1 addition & 1 deletion Binary/Locator/FileSystemLocator.php
Expand Up @@ -60,7 +60,7 @@ public function locate(string $path): string
*/
protected function generateAbsolutePath(string $root, string $path): ?string
{
if (false !== $absolute = realpath($root.DIRECTORY_SEPARATOR.$path)) {
if (false !== $absolute = realpath($root.\DIRECTORY_SEPARATOR.$path)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the backslashes here?

return $absolute;
}

Expand Down
38 changes: 38 additions & 0 deletions Config/Filter/Type/AutoRotate.php
@@ -0,0 +1,38 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class AutoRotate implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @param string $name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets remove this, its redundant

*/
public function __construct(string $name)
{
$this->name = $name;
}

/**
* @return string
*/
public function getName(): string
{
return $this->name;
}
}
103 changes: 103 additions & 0 deletions Config/Filter/Type/Background.php
@@ -0,0 +1,103 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class Background implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @var string
*/
private $color;

/**
* @var string
*/
private $transparency;

/**
* @var string
*/
private $position;

/**
* @var array
*/
private $size;

/**
* @param string $name
* @param string|null $color
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am ok with using a string for the $color, but we should explain here what it is. RRGGBB? with optional transparency?

* @param string|null $transparency
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should state the range of this. 0 to 100? or 0 to 1? or something else?

* @param string|null $position
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that a list of keywords like TOP|LEFT|BOTTOM|RIGHT, or something else?

* @param array $size
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keys x and y? or do we use a value object for this?

*/
public function __construct(
string $name,
string $color = null,
string $transparency = null,
string $position = null,
array $size = []
) {
$this->name = $name;
$this->color = $color;
$this->transparency = $transparency;
$this->position = $position;
$this->size = $size;
}

/**
* @return string
*/
public function getName(): string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Max, this is wrong. If color is optional - use ?string as return type. Also phpdocs which duplicates actual typehints/return types are useless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the hint, will refactor.

{
return $this->name;
}

/**
* @return string|null
*/
public function getColor(): string
{
return $this->color;
}

/**
* @return string|null
*/
public function getTransparency(): string
{
return $this->transparency;
}

/**
* @return string|null
*/
public function getPosition()
{
return $this->position;
}

/**
* @return array
*/
public function getSize(): array
{
return $this->size;
}
}
68 changes: 68 additions & 0 deletions Config/Filter/Type/Crop.php
@@ -0,0 +1,68 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class Crop implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @var array
*/
private $start;

/**
* @var array
*/
private $size;

/**
* @param string $name
* @param array $start start coordinates {x,y}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not provide value object for Point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, will add it

* @param array $size size parameters {width, height}
*/
public function __construct(string $name, array $start, array $size)
{
$this->name = $name;
$this->start = $start;
$this->size = $size;
}

/**
* @return string
*/
public function getName(): string
{
return $this->name;
}

/**
* @return array
*/
public function getStart(): array
{
return $this->start;
}

/**
* @return array
*/
public function getSize(): array
{
return $this->size;
}
}
68 changes: 68 additions & 0 deletions Config/Filter/Type/Downscale.php
@@ -0,0 +1,68 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class Downscale implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @var array
*/
private $max;

/**
* @var float
*/
private $by;

/**
* @param string $name
* @param array $max desired max dimensions {width, height}
* @param float $by sets the "ratio multiple" which initiates a proportional scale operation computed by multiplying all image sides by this value
*/
public function __construct(string $name, array $max = [], float $by = null)
{
$this->name = $name;
$this->max = $max;
$this->by = $by;
}

/**
* @return string
*/
public function getName(): string
{
return $this->name;
}

/**
* @return array
*/
public function getMax(): array
{
return $this->max;
}

/**
* @return float|null
*/
public function getBy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: ?float instead of the docblock

{
return $this->by;
}
}
53 changes: 53 additions & 0 deletions Config/Filter/Type/Flip.php
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class Flip implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @var string
*/
private $axis;

/**
* @param string $name
* @param string $axis possible values are: "x", "horizontal", "y", or "vertical"
*/
public function __construct(string $name, string $axis)
{
$this->name = $name;
$this->axis = $axis;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we validate the values? does imagine provide constants for these?

}

/**
* @return string
*/
public function getName(): string
{
return $this->name;
}

/**
* @return string
*/
public function getAxis(): string
{
return $this->axis;
}
}
38 changes: 38 additions & 0 deletions Config/Filter/Type/Grayscale.php
@@ -0,0 +1,38 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Filter\Type;

use Liip\ImagineBundle\Config\FilterInterface;

final class Grayscale implements FilterInterface
{
/**
* @var string
*/
private $name;

/**
* @param string $name
*/
public function __construct(string $name)
{
$this->name = $name;
}

/**
* @return string
*/
public function getName(): string
{
return $this->name;
}
}