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

[BUG]: Phalcon\Config\ConfigFactory::load return same (first one) config for different files #14584

Closed
konkov-alexey opened this issue Dec 5, 2019 · 4 comments
Assignees
Labels
bug A bug report status: low Low

Comments

@konkov-alexey
Copy link

Describe the bug
if you try to load several configs from files (for instance from php) you get config objects with same values.

To Reproduce
config1.php

return [
    'service1' => [
        'option1' => 'value1',
    ],
];

config2.php

return [
    'service2' => [
        'option2' => 'value2',
    ],
];

then execute

<?php
    $configFactory = new \Phalcon\Config\ConfigFactory();
    $config1 = $configFactory->load('config1.php');
    print_r($config1->toArray());
    $config2 = $configFactory->load('config2.php');
    print_r($config2->toArray());

and result is

Array
(
    [service1] => Array
        (
            [option1] => value1
        )

)
Array
(
    [service1] => Array
        (
            [option1] => value1
        )

)

Expected behavior

Array
(
    [service1] => Array
        (
            [option1] => value1
        )

)
Array
(
    [service2] => Array
        (
            [option2] => value2
        )

)

Details

  • Phalcon version: 4.0.0-rc.3
  • PHP Version: 7.3.12

Additional context
as i understand, the trouble is in Phalcon\Config\ConfigFactory::newInstance() method:

    /**
     * Returns a new Config instance
     */
    public function newInstance(string name, string fileName, var params = null) -> object
    {
        var definition, options;

        this->checkService(name);

        if !isset this->services[name] {
            let definition = this->mapper[name],
                options    = [],
                options[]  = fileName;

            if "json" !== name && "php" !== name {
                let options[] = params;
            }

            let this->services[name] = create_instance_params(definition, options);
        }

        return this->services[name];
    }

if think it should be like

    public function newInstance(string name, string fileName, var params = null) -> object
    {
        var definition, options;

        this->checkService(name);

        let definition = this->mapper[name],
            options    = [],
            options[]  = fileName;

        if "json" !== name && "php" !== name {
            let options[] = params;
        }

        return create_instance_params(definition, options);
    }
@konkov-alexey
Copy link
Author

the same problem, i guess will be with Phalcon\Cache\AdapterFactory::newInstance and may be some other extended from Phalcon\Factory\AbstractFactory.

implemented logic suppose that there should be only one instance of each adapter, created with each instance of AdapterFactory, so you cannot get several instances of same adapter but with different instance options with one AdapterFactory instance .

@ruudboon ruudboon added the 4.0 label Dec 6, 2019
@ruudboon ruudboon self-assigned this Dec 6, 2019
@niden
Copy link
Member

niden commented Dec 6, 2019

This is because the implementation of the factories uses an internal mapper so that you get the same instance no matter how many times you call newInstance in the same request.

I will sort this out tomorrow. Not a difficult fix.

@niden niden mentioned this issue Dec 7, 2019
5 tasks
niden added a commit that referenced this issue Dec 7, 2019
niden added a commit that referenced this issue Dec 7, 2019
niden added a commit that referenced this issue Dec 7, 2019
niden added a commit that referenced this issue Dec 7, 2019
@niden
Copy link
Member

niden commented Dec 7, 2019

Addressed in #14592

@niden niden closed this as completed Dec 7, 2019
@konkov-alexey
Copy link
Author

thanks!

@niden niden added bug A bug report status: low Low and removed Bug - Low labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: low Low
Projects
None yet
Development

No branches or pull requests

3 participants