diff --git a/Controller/SettingsController.php b/Controller/SettingsController.php index 3f0b159..1105ed9 100644 --- a/Controller/SettingsController.php +++ b/Controller/SettingsController.php @@ -2,7 +2,6 @@ namespace Craue\ConfigBundle\Controller; -use Craue\ConfigBundle\CacheAdapter\CacheAdapterInterface; use Craue\ConfigBundle\Entity\SettingInterface; use Craue\ConfigBundle\Form\ModifySettingsForm; use Doctrine\ORM\EntityManagerInterface; @@ -22,7 +21,7 @@ */ class SettingsController extends AbstractController { - public function modifyAction(CacheAdapterInterface $cache, FormFactoryInterface $formFactory, Request $request, + public function modifyAction(FormFactoryInterface $formFactory, Request $request, SessionInterface $session, Environment $twig, EntityManagerInterface $em, TranslatorInterface $translator) { $repo = $em->getRepository($this->container->getParameter('craue_config.entity_name')); $allStoredSettings = $repo->findAll(); @@ -37,14 +36,6 @@ public function modifyAction(CacheAdapterInterface $cache, FormFactoryInterface $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - // update the cache - foreach ($formData['settings'] as $formSetting) { - $storedSetting = $this->getSettingByName($allStoredSettings, $formSetting->getName()); - if ($storedSetting !== null) { - $cache->set($storedSetting->getName(), $storedSetting->getValue()); - } - } - $em->flush(); if ($session instanceof Session) { @@ -80,19 +71,4 @@ protected function getSections(array $settings) { return $sections; } - /** - * @param SettingInterface[] $settings - * @param string $name - * @return SettingInterface|null - */ - protected function getSettingByName(array $settings, $name) { - foreach ($settings as $setting) { - if ($setting->getName() === $name) { - return $setting; - } - } - - return null; - } - } diff --git a/DependencyInjection/CraueConfigExtension.php b/DependencyInjection/CraueConfigExtension.php index 5e45020..29ead0f 100644 --- a/DependencyInjection/CraueConfigExtension.php +++ b/DependencyInjection/CraueConfigExtension.php @@ -24,6 +24,7 @@ class CraueConfigExtension extends Extension implements PrependExtensionInterfac public function load(array $configs, ContainerBuilder $container) : void { $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); $loader->load('controller.xml'); + $loader->load('event_listener.xml'); $loader->load('form.xml'); $loader->load('twig.xml'); $loader->load('util.xml'); diff --git a/EventListener/SettingUpdateListener.php b/EventListener/SettingUpdateListener.php new file mode 100644 index 0000000..3cad31e --- /dev/null +++ b/EventListener/SettingUpdateListener.php @@ -0,0 +1,43 @@ + + * @copyright 2011-2023 Christian Raue + * @license http://opensource.org/licenses/mit-license.php MIT License + */ +class SettingUpdateListener { + + /** + * @var CacheAdapterInterface + */ + private $cache; + + public function __construct(?CacheAdapterInterface $cache = null) { + $this->cache = $cache ?? new NullAdapter(); + } + + // TODO add `PostUpdateEventArgs` type-hint as soon as doctrine/orm >= 2.13 is required + public function postUpdate($eventArgs) : void { + assert($eventArgs instanceof LifecycleEventArgs); + + $entity = $eventArgs->getObject(); + + if (!$entity instanceof SettingInterface) { + return; + } + + $this->updateCache($entity); + } + + private function updateCache(SettingInterface $setting) : void { + $this->cache->set($setting->getName(), $setting->getValue()); + } + +} diff --git a/Resources/config/event_listener.xml b/Resources/config/event_listener.xml new file mode 100644 index 0000000..f2bd449 --- /dev/null +++ b/Resources/config/event_listener.xml @@ -0,0 +1,16 @@ + + + + + + + + + diff --git a/Tests/Controller/SettingsControllerIntegrationTest.php b/Tests/Controller/SettingsControllerIntegrationTest.php index 60f234a..2c47e93 100644 --- a/Tests/Controller/SettingsControllerIntegrationTest.php +++ b/Tests/Controller/SettingsControllerIntegrationTest.php @@ -132,9 +132,8 @@ public function testModifyAction_changeValue_cacheUsage($platform, $config, $req ]); $this->assertTrue($cache->has('name1')); - $this->assertTrue($cache->has('name2')); + $this->assertFalse($cache->has('name2')); $this->assertSame('new-value1', $cache->get('name1')); - $this->assertSame('value2', $cache->get('name2')); } public function dataModifyAction_changeValue_cacheUsage() { diff --git a/Tests/Controller/SettingsControllerUnitTest.php b/Tests/Controller/SettingsControllerUnitTest.php index 0563077..f27159a 100644 --- a/Tests/Controller/SettingsControllerUnitTest.php +++ b/Tests/Controller/SettingsControllerUnitTest.php @@ -40,26 +40,4 @@ public function dataGetSections() { ]; } - /** - * @dataProvider dataGetSettingByName - */ - public function testGetSettingByName(array $settings, $name, $expectedResult) { - $controller = new SettingsController(); - $method = new \ReflectionMethod($controller, 'getSettingByName'); - $method->setAccessible(true); - - $this->assertSame($expectedResult, $method->invoke($controller, $settings, $name)); - } - - public function dataGetSettingByName() { - $setting1 = Setting::create('name1'); - $setting2 = Setting::create('name2'); - - return [ - [[$setting1], 'name1', $setting1], - [[$setting1, $setting2], 'name2', $setting2], - [[$setting1, $setting2], 'name3', null], - ]; - } - } diff --git a/Tests/EventListener/SettingUpdateListenerTest.php b/Tests/EventListener/SettingUpdateListenerTest.php new file mode 100644 index 0000000..fd8d05d --- /dev/null +++ b/Tests/EventListener/SettingUpdateListenerTest.php @@ -0,0 +1,68 @@ + + * @copyright 2011-2023 Christian Raue + * @license http://opensource.org/licenses/mit-license.php MIT License + */ +class SettingUpdateListenerTest extends TestCase { + + public function testPostUpdate() : void { + $cache = $this->createMock(CacheAdapterInterface::class); + $listener = new SettingUpdateListener($cache); + + $setting = $this->getMockBuilder(SettingInterface::class)->onlyMethods(['getName', 'getValue'])->getMockForAbstractClass(); + $name = 'name'; + $newValue = 'new-value'; + + $setting->expects($this->once()) + ->method('getName') + ->willReturn($name) + ; + + $setting->expects($this->once()) + ->method('getValue') + ->willReturn($newValue) + ; + + $cache->expects($this->once()) + ->method('set') + ->with($name, $newValue) + ; + + $listener->postUpdate($this->getPostUpdateEventArgs($setting)); + } + + public function testPostUpdate_entityIsNotSetting() : void { + $cache = $this->createMock(CacheAdapterInterface::class); + $listener = new SettingUpdateListener($cache); + + $cache->expects($this->never()) + ->method('set') + ; + + $listener->postUpdate($this->getPostUpdateEventArgs(new \stdClass())); + } + + // TODO remove as soon as doctrine/orm >= 2.13 is required + private function getPostUpdateEventArgs(object $object) : LifecycleEventArgs { + $em = $this->createMock(EntityManagerInterface::class); + + if (!class_exists(PostUpdateEventArgs::class)) { + return new LifecycleEventArgs($object, $em); + } + + return new PostUpdateEventArgs($object, $em); + } + +} diff --git a/Tests/Util/ConfigIntegrationTest.php b/Tests/Util/ConfigIntegrationTest.php index a2d12ff..e82c733 100644 --- a/Tests/Util/ConfigIntegrationTest.php +++ b/Tests/Util/ConfigIntegrationTest.php @@ -75,6 +75,30 @@ public function dataCacheUsage() { return $testData; } + /** + * Ensure that the cache is updated when updating a setting entity directly. + * + * @dataProvider dataCacheUsage + */ + public function testCacheUpdateOnEntityUpdate($platform, $config, $requiredExtension, $environment) : void { + $this->initClient($requiredExtension, ['environment' => $environment . '_' . $platform, 'config' => $config]); + + $cache = $this->getService('craue_config_cache_adapter'); + $cache->clear(); + + $setting = Setting::create('name', 'value'); + $this->persistSetting($setting); + + $this->assertFalse($cache->has($setting->getName())); + + // update the entity directly + $setting->setValue('new-value2'); + $this->getEntityManager()->flush(); + + $this->assertTrue($cache->has($setting->getName())); + $this->assertSame('new-value2', $cache->get($setting->getName())); + } + /** * Ensure that a custom config class can actually be used with a custom model class. * diff --git a/Tests/Util/ConfigUnitTest.php b/Tests/Util/ConfigUnitTest.php index 9e421c5..ab4ec64 100644 --- a/Tests/Util/ConfigUnitTest.php +++ b/Tests/Util/ConfigUnitTest.php @@ -101,9 +101,8 @@ public function testSet() { $config->setEntityManager($this->createEntityManagerMock($this->createEntityRepositoryMock(['findOneBy' => $setting]))); - $cache->expects($this->once()) - ->method('set') - ->with($setting->getName(), $newValue) + $cache->expects($this->never()) + ->method($this->anything()) ; $setting->expects($this->once()) @@ -138,9 +137,8 @@ public function testSetMultiple() { $setting->getName() => $newValue, ]; - $cache->expects($this->once()) - ->method('setMultiple') - ->with($settingsKeyValuePairs) + $cache->expects($this->never()) + ->method($this->anything()) ; $setting->expects($this->once()) diff --git a/Util/Config.php b/Util/Config.php index 9c5ba40..5e7469b 100644 --- a/Util/Config.php +++ b/Util/Config.php @@ -99,7 +99,7 @@ public function set($name, $value) { $setting->setValue($value); $this->em->flush(); - $this->cache->set($name, $value); + // cache is updated in SettingUpdateListener } /** @@ -123,7 +123,7 @@ public function setMultiple(array $newSettings) { $this->em->flush(); - $this->cache->setMultiple($newSettings); + // cache is updated in SettingUpdateListener } /**