Add service to use config parameters in custom storage like DB #10357

kuzmany · 2021-08-23T10:55:56Z

Q	A
Branch?	"features" for all features, enhancements and bug fixes (until 3.3.0 is released)
Bug fix?	yes/no
New feature?	yes/no
Deprecations?	yes/no
BC breaks?	yes/no
Automated tests included?	yes/no
Related user documentation PR URL	mautic/mautic-documentation#...
Related developer documentation PR URL	mautic/developer-documentation#...
Issue(s) addressed	Fixes #...

Description:

This PR allow developer create own storage for parameters - by default store in local.php. Some enviroment infrastructure do not allow change it static files due server replication. You can use database or redis or enything else.

Example of service for parameters storage:

            'mautic.parameters.storage.database' => [
                'class'     => \Mautic\CoreBundle\ParametersStorage\Database\DatabaseParametersStorage::class,
                'arguments' => [
                    'mautic.cache.provider',
                ],
                'tag'       => 'mautic.parameters.storage.service',
            ],

Then in in parameters_local.php you set

\Mautic\CoreBundle\ParametersStorage\ParametersStorage::PARAMETERS_STORAGE => 'mautic.parameters.storage.database',

This PR also include Add support for database cache provider + allow use multiple cache provider in code

Cache component (https://developer.mautic.org/#cache-bundle) in Mautic support at the moment filesystem, memcached a redis mechanism. This PR add database cache adapter (https://symfony.com/doc/current/components/cache/adapters/pdo_doctrine_dbal_adapter.html)

Also this allow use multiple cache provider in your code.

$cache = $this->get('mautic.cache.provider');
$item = $cache->getItem('test_tagged_Item');
$item->set('yesa!!!');

and

$cache = $this->get('mautic.cache.provider')->getCacheAdapter('mautic.cache.adapter.db');
$item = $cache->getItem('test_tagged_Item');
$item->set('yesa!!!');

Just setup in parameters_local.php

'cache_adapter' => 'mautic.cache.adapter.db',

You can also change there timeout to never expired data

        'cache_adapter_db'        => [ ],

Steps to test this PR:

Load up this PR

codecov · 2021-08-23T11:06:34Z

Codecov Report

Merging #10357 (d3bd064) into 4.x (21430d8) will increase coverage by 0.01%.
The diff coverage is 93.02%.

❗ Current head d3bd064 differs from pull request most recent head 1db5f68. Consider uploading reports for the commit 1db5f68 to get more accurate results

@@             Coverage Diff              @@
##                4.x   #10357      +/-   ##
============================================
+ Coverage     42.14%   42.16%   +0.01%     
- Complexity    34533    34545      +12     
============================================
  Files          2062     2065       +3     
  Lines        111560   111584      +24     
============================================
+ Hits          47021    47045      +24     
  Misses        64539    64539

Impacted Files	Coverage Δ
...ndencyInjection/Compiler/ParametersStoragePass.php	`83.33% <83.33%> (ø)`
...CoreBundle/ParametersStorage/ParametersStorage.php	`90.00% <90.00%> (ø)`
...ParametersStorage/Local/LocalParametersStorage.php	`91.66% <91.66%> (ø)`
...ndles/ConfigBundle/Controller/ConfigController.php	`57.60% <100.00%> (-0.61%)`	⬇️
...Bundle/DependencyInjection/MauticCoreExtension.php	`91.82% <100.00%> (+0.21%)`	⬆️
app/bundles/CoreBundle/MauticCoreBundle.php	`100.00% <100.00%> (ø)`

RCheesley · 2021-08-30T17:53:13Z

@kuzmany just updated the base for you to 4.x as it's a feature PR.

alanhartless

@kuzmany I made some comments just by reading the code. I have not tested it. It needs code coverage for the new services and that existing services still work as expected.

alanhartless · 2021-10-18T18:11:54Z

app/bundles/CacheBundle/Cache/Adapter/DatabaseTagAwareAdapter.php

+
+        parent::__construct(
+            new \Symfony\Component\Cache\Adapter\PdoAdapter($connection, (string) $namespace, $lifetime, $options),
+            new \Symfony\Component\Cache\Adapter\PdoAdapter($connection, (string) $namespace, $lifetime, $options)


Did you mean to duplicate this?

alanhartless · 2021-10-18T18:16:59Z

app/bundles/CacheBundle/Cache/CacheProvider.php

    {
        if (is_null($this->psr16)) {
-            $this->psr16 = new Psr6Cache($this->getCacheAdapter());
+            $this->psr16 = new Psr6Cache($this->getCacheAdapter($cacheAdapter));


I think this will cause issues when the cache provider is used for caching (i.e. Redis) and the database (for config) because first to call the method wins resulting subsequent to use something other than the cache adaptor configured.

Consider using a complete new service for this instead of sharing the same as the cache provider. DatabaseParametersStorage could potentially use the same class but should be defined as a new/separate service.

alanhartless · 2021-10-18T18:17:55Z

app/bundles/CoreBundle/Config/config.php

+            'mautic.parameters.storage.database' => [
+                'class'     => \Mautic\CoreBundle\ParametersStorage\Database\DatabaseParametersStorage::class,
+                'arguments' => [
+                    'mautic.cache.provider',


See my comment above about using a new service instead of sharing mautic.cache.provider

alanhartless · 2021-10-18T18:28:53Z

app/bundles/CoreBundle/ParametersStorage/Database/DatabaseParametersStorage.php

+
+    public function write(array $parameters): void
+    {
+        $this->simpleCache->set(ParametersStorage::PARAMETERS_STORAGE, $parameters);


I'm torn on this. I see the benefit of quicker coding by using the cache provider to do this but I also think it's a bit risky because important configuration options will be stored in the same location as cached items. It'll need to be called out that using the DB to store parameters means you cannot reset or truncate the database cache blindly. If using a different service than the shared mautic.cache.provider, maybe you could use a new table just for config parameters as well to help protect from that scenario?

@alanhartless thank you for your comments. Do you mean use cache service with store to another table like cache_items or do you mean create new table as part of Mautic?

A new table that stores just config values that doesn't get tangled up in other cached items because the mautic.cache.provider service is shared for both.

alanhartless · 2021-10-18T18:33:31Z

app/bundles/InstallBundle/Install/InstallService.php

-        // if db_driver and mailer_from_name are present then it is assumed all the steps of the installation have been
-        // performed; manually deleting these values or deleting the config file will be required to re-enter
-        // installation.
-        if (empty($params['db_driver']) || empty($params['mailer_from_name'])) {


Was this check moved somewhere else? If not, then was the installer tested with the default file based config? It looks like this method is used in a few places so I'm not sure of the full impact of removing this nor if there is really any value in having it now.

…ovider in code mautic#10513

…cache provider in code mautic#10513" This reverts commit 09d954b

kuzmany · 2024-02-14T06:44:25Z

Close, we need release M5 version of the solution

Add ParametersStorage service for starts

a886b0d

kuzmany added WIP PR's that are not ready for review and are currently in progress enhancement Any improvement to an existing feature or functionality labels Aug 23, 2021

cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Aug 23, 2021

RCheesley marked this pull request as draft August 24, 2021 16:57

kuzmany changed the title ~~Add services to use cofnig parameters in storage like DB~~ Add services to use config parameters in custom storage like DB Aug 26, 2021

Add LocalParametersStorage

3bb4f25

RCheesley changed the base branch from features to 4.x August 30, 2021 17:52

Merge branch '4.x' into configuration-to-database

13597cb

kuzmany changed the title ~~Add services to use config parameters in custom storage like DB~~ Add service to use config parameters in custom storage like DB Oct 6, 2021

Add isValid to ParametersStorageInterface.php

d3bd064

kuzmany mentioned this pull request Oct 7, 2021

Add support for database cache provider + allow use multiple cache provider in code #10513

Closed

alanhartless suggested changes Oct 18, 2021

View reviewed changes

kuzmany added 13 commits January 11, 2022 13:29

Fix validation for configuration save

810fe2a

Add database storage

a87e7f1

Add load parameters from storage to CoreParametersHelper

9107da1

Add support for database cache provider + allow use multiple cache pr…

cf87fc3

…ovider in code mautic#10513

Revert back to changes for RestrictedParameters

ee02e9d

Remove junk comments

c97d209

Remove not used use statement

8f7c3b3

Fix database cache provider for DatabaseParametersStorage

1cddd76

Add options for database cache

56977e4

Revert "Add support for database cache provider + allow use multiple …

edeaa03

…cache provider in code mautic#10513" This reverts commit 09d954b

Revert back checkIfInstalled

9081618

Remove DatabaseParametersStorage

25ddcd3

Remove mautic.parameters.storage.database from config

6f056b2

kuzmany force-pushed the configuration-to-database branch from 5123b1e to 6f056b2 Compare January 11, 2022 12:29

Fix get parameters from getParameterBag

8c7ed3d

Check just db_driver for isInstalled logic

1db5f68

RCheesley added the needs-rebase PR's that need to be rebased label Aug 4, 2023

kuzmany closed this Feb 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add service to use config parameters in custom storage like DB #10357

Add service to use config parameters in custom storage like DB #10357

kuzmany commented Aug 23, 2021 •

edited

Loading

codecov bot commented Aug 23, 2021 •

edited

Loading

RCheesley commented Aug 30, 2021

alanhartless left a comment

alanhartless Oct 18, 2021

alanhartless Oct 18, 2021

alanhartless Oct 18, 2021

alanhartless Oct 18, 2021

kuzmany Oct 18, 2021 •

edited

Loading

alanhartless Oct 18, 2021

alanhartless Oct 18, 2021

kuzmany commented Feb 14, 2024

Add service to use config parameters in custom storage like DB #10357

Add service to use config parameters in custom storage like DB #10357

Conversation

kuzmany commented Aug 23, 2021 • edited Loading

Description:

Steps to test this PR:

codecov bot commented Aug 23, 2021 • edited Loading

Codecov Report

RCheesley commented Aug 30, 2021

alanhartless left a comment

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

kuzmany Oct 18, 2021 • edited Loading

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

alanhartless Oct 18, 2021

Choose a reason for hiding this comment

kuzmany commented Feb 14, 2024

kuzmany commented Aug 23, 2021 •

edited

Loading

codecov bot commented Aug 23, 2021 •

edited

Loading

kuzmany Oct 18, 2021 •

edited

Loading