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 2 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
57 changes: 57 additions & 0 deletions Config/Filter.php
@@ -0,0 +1,57 @@
<?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;

final class Filter implements FilterInterface
Copy link
Member

Choose a reason for hiding this comment

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

once you implemented all filters, this will become obsolete, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{
/**
* @var string
*/
private $name;

/**
* @var array
*/
private $options = [];

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

/**
* @param string $name
*/
public function setName(string $name): void
Copy link
Member

Choose a reason for hiding this comment

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

i'd rather use constructor arguments than setters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, will refactor it.

{
$this->name = $name;
}

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

/**
* @param array $options
*/
public function setOptions(array $options): void
{
$this->options = $options;
}
}
45 changes: 45 additions & 0 deletions Config/FilterBuilder.php
@@ -0,0 +1,45 @@
<?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;

use Liip\ImagineBundle\Factory\Config\FilterFactory;

final class FilterBuilder implements FilterBuilderInterface
{
/**
* @var FilterFactory
*/
private $filterFactory;

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

/**
* @param string $filterName
* @param array $filterData
*
* @return FilterInterface
*/
public function build(string $filterName, array $filterData): FilterInterface
{
$filter = $this->filterFactory->create();
$filter->setName($filterName);
$filter->setOptions($filterData);

return $filter;
}
}
23 changes: 23 additions & 0 deletions Config/FilterBuilderInterface.php
@@ -0,0 +1,23 @@
<?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;

interface FilterBuilderInterface
{
/**
* @param string $filterName
* @param array $filterData
*
* @return FilterInterface
*/
public function build(string $filterName, array $filterData): FilterInterface;
}
35 changes: 35 additions & 0 deletions Config/FilterInterface.php
@@ -0,0 +1,35 @@
<?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;

interface FilterInterface
Copy link
Member

Choose a reason for hiding this comment

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

i would like to do a phpdoc that explains what a filter is (the definition for one image transformation operation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dbu could you provide an example ? cause I'm not sure that I understand this comment.

Copy link
Member

Choose a reason for hiding this comment

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

/**
 * A filter contains the configuration for an image transformation operation.
 *
 * The type of operation is defined by the filter class. The operation parameters are specific to
 * the filter.
 */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{
/**
* @return string
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 say what the name is for. is it only for debugging purposes? do we match actual filters by filter class or by filter name?

i think filters should be matched by filter name, to allow having the same class represent different filters.

Copy link
Member

Choose a reason for hiding this comment

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

^

*/
public function getName(): string;

/**
* @return array
*/
public function getOptions(): array;

/**
* @param string $name
*/
public function setName(string $name): void;
Copy link
Member

Choose a reason for hiding this comment

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

imo the interface should not define setters. the whole definition is a read-only thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

completely agree with you :)


/**
* @param array $options
*/
public function setOptions(array $options): void;
}
106 changes: 106 additions & 0 deletions Config/FilterSet.php
@@ -0,0 +1,106 @@
<?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;

use Liip\ImagineBundle\Exception\InvalidArgumentException;

final class FilterSet implements FilterSetInterface
Copy link
Member

Choose a reason for hiding this comment

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

as discussed, rename this to Stack

{
/**
* @var string
*/
private $name;

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

/**
* @var int
*/
private $quality;

/**
* @var FilterInterface[]
*/
private $filters = [];

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

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

/**
* @return int
*/
public function getQuality(): int
Copy link
Member

Choose a reason for hiding this comment

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

this should have a phpdoc explaining what it is. (resp, as rob says the jpg quality and the png compression level values should have that doc, to explain the value range)

{
return $this->quality;
}

/**
* @return FilterInterface[]
*/
public function getFilters(): array
{
return $this->filters;
}

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

/**
* @param string|null $dataLoader
*/
public function setDataLoader($dataLoader): void
{
$this->dataLoader = (string) $dataLoader;
}

/**
* @param int $quality
*/
public function setQuality(int $quality): void
{
$this->quality = $quality;
}

/**
* @param FilterInterface[] $filters
*/
public function setFilters(array $filters): void
{
foreach ($filters as $filter) {
if (!($filter instanceof FilterInterface)) {
throw new InvalidArgumentException('Unknown filter provided.');
}
}
$this->filters = $filters;
}
}
61 changes: 61 additions & 0 deletions Config/FilterSetBuilder.php
@@ -0,0 +1,61 @@
<?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;

use Liip\ImagineBundle\Factory\Config\FilterSetFactory;

final class FilterSetBuilder implements FilterSetBuilderInterface
{
/**
* @var FilterSetFactory
*/
private $filterSetFactory;

/**
* @var FilterBuilderInterface
*/
private $filterBuilder;

/**
* @param FilterSetFactory $filterSetFactory
* @param FilterBuilderInterface $filterBuilder
*/
public function __construct(FilterSetFactory $filterSetFactory, FilterBuilderInterface $filterBuilder)
{
$this->filterSetFactory = $filterSetFactory;
$this->filterBuilder = $filterBuilder;
}

/**
* @param string $filterSetName
* @param array $filterSetData
*
* @return FilterSetInterface
*/
public function build(string $filterSetName, array $filterSetData): FilterSetInterface
{
$filterSet = $this->filterSetFactory->create();
$filterSet->setName($filterSetName);
$filterSet->setDataLoader($filterSetData['data_loader']);
$filterSet->setQuality($filterSetData['quality']);

if (!empty($filterSetData['filters'])) {
$filters = [];
foreach ($filterSetData['filters'] as $filterName => $filterData) {
$filters[] = $this->filterBuilder->build($filterName, $filterData);
}
$filterSet->setFilters($filters);
}

return $filterSet;
}
}
23 changes: 23 additions & 0 deletions Config/FilterSetBuilderInterface.php
@@ -0,0 +1,23 @@
<?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;

interface FilterSetBuilderInterface
{
/**
* @param string $filterSetName
* @param array $filterSetData
*
* @return FilterSetInterface
*/
public function build(string $filterSetName, array $filterSetData): FilterSetInterface;
}