8000 [PropertyAccess] Allow custom methods on property accesses by lrlopez · Pull Request #18016 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[PropertyAccess] Allow custom methods on property accesses #18016

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.

Sign up for GitHub

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

Closed
wants to merge 18 commits into from
Closed

[PropertyAccess] Allow custom methods on property accesses #18016

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
af3a824
[PropertyAccess] Allow custom methods on property accesses
lrlopez Mar 4, 2016
5b82237
Added support for YAML and XML mapping
lrlopez Mar 6, 2016
00a26eb
Renamed Attribute to Property on most classes and methods
lrlopez Mar 16, 2016
e4aaac9
Annotation renamed to @Property
lrlopez Mar 17, 2016
e46df98
Added method annotation for virtual properties
lrlopez Mar 21, 2016
5f206e6
Renamed "attribute" to "property" in XSD and YAML
lrlopez Mar 21, 2016
94e6157
Fixed the metadata caching mess
lrlopez Mar 21, 2016
09707ad
Replaced metadata caching with standard PSR-6 cache pool
lrlopez Mar 22, 2016
d02d93a
Added support to metadata factories in PropertyAccesorBuilder
lrlopez Mar 25, 2016
748b894
Fix outdated type hint in LazyLoadingMetadataFactory
lrlopez Mar 29, 2016
688cbda
Fixed typo in variable name
lrlopez Apr 1, 2016
003efdb
Added 'Property' prefix to method annotations
lrlopez Apr 2, 2016
dc5cacb
Renamed 'propertiesMetadata' to 'propertyMetadataCollection' in 'Clas…
lrlopez Apr 2, 2016
1a35d45
Minor rebase fixes
lrlopez Jul 3, 2016
1e29523
Addressed @xabbuh and @fabpot comments
lrlopez Jul 3, 2016
7c6a79c
Make fabbot.io happy :)
lrlopez Jul 3, 2016
d0bd777
Fixed class name clash in FrameworkExtension
lrlopez Jul 3, 2016
6b7eeff
Revert unintended change in composer.json
lrlopez Jul 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Replaced metadata caching with standard PSR-6 cache pool
  • Loading branch information
@lrlopez
lrlopez committed Jul 3, 2016
commit 09707add7aa25cd52910e9197d357a01bcedf988
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1538,3 +1538,4 @@ Symfony is the result of the work of many people who made the code better
- Erik Saunier (snickers)
- Matej Žilák (teo_sk)
- Vladislav Vlastovskiy (vlastv)
- Luis Ramón López (lrlopez)
Copy link
Contributor

Choose a reason for hiding this comment

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

this file autogenerates on each release ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had no idea 😅 Thanks for the info! I'll take it out then

5 changes: 0 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,11 +987,6 @@ private function registerPropertyAccessConfiguration(array $config, ContainerBui
$chainLoader->replaceArgument(0, $serializerLoaders);

if (isset($config['cache']) && $config['cache']) {
Copy link
Member

Choose a reason for hiding this comment

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

It should be delayed until #17868 is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, it makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure #17868 will be merged before the 3.1 feature freeze. Could we skip this and work on it after the new Cache component is ready?

Copy link
Member

Choose a reason for hiding this comment

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

It should use the new system.cache infrastructure: http://symfony.com/blog/new-in-symfony-3-1-cache-component

$container->setParameter(
'property_access.mapping.cache.prefix',
'property_access_'.$this->getKernelRootHash($container)
);

$container->getDefinition('property_access.mapping.class_metadata_factory')->replaceArgument(
1, new Reference($config['cache'])
);
Expand Down
15 changes: 0 additions & 15 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/property_access.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<parameters>
<parameter key="property_access.mapping.cache.prefix" />
</parameters>

<services>

<!-- Loader -->
<service id="property_access.mapping.chain_loader" class="Symfony\Component\PropertyAccess\Mapping\Loader\LoaderChain" public="false">
<argument type="collection" />
Expand All @@ -21,16 +16,6 @@
<argument>null</argument>
</service>

<service id="property_access.mapping.cache.doctrine.apc" class="Symfony\Component\PropertyAccess\Mapping\Cache\DoctrineCache" public="false">
<argument type="service">
<service class="Doctrine\Common\Cache\ApcCache">
<call method="setNamespace">
<argument>%property_access.mapping.cache.prefix%</argument>
</call>
</service>
</argument>
</service>

<service id="property_accessor" class="Symfony\Component\PropertyAccess\PropertyAccessor" >
<argument /> <!-- magicCall, set by the extension -->
<argument /> <!-- throwExceptionOnInvalidIndex, set by the extension -->
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
'magic_call' => false,
'throw_exception_on_invalid_index' => false,
'enable_annotations' => true,
'cache' => 'property_access.mapping.cache.doctrine.apc',
),
'serializer' => array(
'enabled' => true,
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ framework:
enable_annotations: true
magic_call: false
throw_exception_on_invalid_index: false
cache: property_access.mapping.cache.doctrine.apc
ide: file%%link%%format
request:
formats:
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/PropertyAccess/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
CHANGELOG
=========

3.1.0
Copy link
Member

Choose a reason for hiding this comment

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

should be 3.2 now

------
* added custom method calling for properties.
Copy link
Member

Choose a reason for hiding this comment

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

This is unclear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll add a more thoughtful explanation


2.7.0
------

Expand Down
45 changes: 0 additions & 45 deletions src/Symfony/Component/PropertyAccess/Mapping/Cache/CacheInterface.php
View file
Open in desktop

This file was deleted.

69 changes: 0 additions & 69 deletions src/Symfony/Component/PropertyAccess/Mapping/Cache/DoctrineCache.php
View file
Open in desktop

This file was deleted.

78 changes: 0 additions & 78 deletions src/Symfony/Component/PropertyAccess/Mapping/Cache/Psr6Cache.php
View file
Open in desktop

This file was deleted.

32 changes: 24 additions & 8 deletions src/Symfony/Component/PropertyAccess/Mapping/Factory/LazyLoadingMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

namespace Symfony\Component\PropertyAccess\Mapping\Factory;

use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\PropertyAccess\Exception\NoSuchMetadataException;
use Symfony\Component\PropertyAccess\Mapping\Cache\CacheInterface;
use Symfony\Component\PropertyAccess\Mapping\ClassMetadata;
use Symfony\Component\PropertyAccess\Mapping\Loader\LoaderInterface;

Expand Down Expand Up @@ -62,11 +62,11 @@ class LazyLoadingMetadataFactory implements MetadataFactoryInterface
/**
* Creates a new metadata factory.
*
* @param LoaderInterface|null $loader The loader for configuring new metadata
* @param CacheInterface|null $cache The cache for persisting metadata
* between multiple PHP requests
* @param LoaderInterface|null $loader The loader for configuring new metadata
* @param CacheItemPoolInterface|null $cache The PSR-6 cache for persisting metadata
* between multiple PHP requests
*/
public function __construct(LoaderInterface $loader = null, CacheInterface $cache = null)
public function __construct(LoaderInterface $loader = null, CacheItemPoolInterface $cache = null)
Copy link
Member

Choose a reason for hiding this comment

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

The cache would be better as a decorator.

Copy link
Contributor Author
@lrlopez lrlopez Jul 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I just replicated the Validator way of accessing metadata to ease a future refactoring (i.e. when Symfony gets its own Metadata component) so I wouldn't like to diverge too much from the established implementation.

@dunglas, Do you still want me to implement the cache as a decorator?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for a decorator, this would make this class easier to understand.

{
$this->loader = $loader;
$this->cache = $cache;
Expand Down Expand Up @@ -99,8 +99,11 @@ public function getMetadataFor($value)
return $this->loadedClasses[$class];
}

if (null !== $this->cache && false !== ($this->loadedClasses[$class] = $this->cache->read($class))) {
return $this->loadedClasses[$class];
if (null !== $this->cache) {
$item = $this->cache->getItem($this->escapeClassName($class));
if ($item->isHit()) {
return $this->loadedClasses[$class] = $item->get();
}
}

if (!class_exists($class) && !interface_exists($class)) {
Expand All @@ -124,7 +127,8 @@ public function getMetadataFor($value)
}

if (null !== $this->cache) {
$this->cache->write($metadata);
$item->set($metadata);
$this->cache->save($item);
}

return $this->loadedClasses[$class] = $metadata;
Expand All @@ -147,4 +151,16 @@ public function hasMetadataFor($value)

return false;
}

/**
* Replaces backslashes by dots in a class name.
*
* @param string $class
*
* @return string
*/
private function escapeClassName($class)
{
return str_replace('\\', '.', $class);
}
}
Loading
0