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

Separate config file for apigility per module #64

Open
michalbundyra opened this issue Jan 10, 2020 · 4 comments
Open

Separate config file for apigility per module #64

michalbundyra opened this issue Jan 10, 2020 · 4 comments

Comments

@michalbundyra
Copy link
Member

Hi,

I tried to use Apigility in an existing module where the config file has stuff like this:

<?php
namespace Demo; // <== THIS RIGHT HERE

use Demo\Controller\IndexController;

return [
    // ...
    'log' => [
        'log.demo' => [
            'writers' => [
                [  // <==== THIS RIGHT HERE
                    'name' => 'stream',
                    'priority' => 1000,
                    'options' => [
                        'stream' => 'data/logs/demo.log',
                    ],
                ],
            ],
        ],
    ],
    'translator' => [
        'locale' => 'en_US',
        'translation_file_patterns' => [
            [
                'type' => 'gettext',
                'base_dir' => __DIR__ . '/../language', // <==== THIS RIGHT HERE
                'pattern' => '%s.mo',
            ],
        ],
    ],
    'controllers' => [
        'invokables' => [
            'Demo\Controller\Index' => IndexController::class,  // <== THIS RIGHT HERE
        ],
    ],
    // ..
];

After using the admin interface it naturally added stuff of it's own to the config and in the process altered the config file. I understand why and how ZF\Configuration does what it does.

  • Changed short array syntax [] to array() - this can be turned off
  • Added number keys to arrays that didn't specify them
  • Converted IndexController::class to it's string representation
  • Converted DIR to the actual directory.
  • Removes the namespace at the top

All of this also messes with your VCS of choice and you either have to revert some changes back or commit the changed config with the changes Apigility introduced.

<?php
// <== missing namespace Demo;

return [
    // ...
    'log' => [
        'log.demo' => [
            'writers' => [
                0 => [  // <==== THIS RIGHT HERE
                    'name' => 'stream',
                    'priority' => 1000,
                    'options' => [
                        'stream' => 'data/logs/demo.log',
                    ],
                ],
            ],
        ],
    ],
    'translator' => [
        'locale' => 'en_US',
        'translation_file_patterns' => [
           0 => [ // <== THIS RIGHT HERE
                'type' => 'gettext',
                'base_dir' => '/some/actual/path/../language', // <==== THIS RIGHT HERE
                'pattern' => '%s.mo',
            ],
        ],
    ],
    'controllers' => [
        'invokables' => [
            'Demo\Controller\Index' => 'Demo\\Controller\\IndexController',  // <== THIS RIGHT HERE
        ],
    ],
    // ..
];

I propose that Apigility uses a different file for it's config. The reason this makes sense to me is that we are already implementing a custom interface ApigilityProviderInterface in the module. The module class getConfig would have to merge these two together for it to work.

Implementing this could be done in several ways. Some of which are:

  1. convention + manual loading: Demo/config/apigility.config.php and array_merge in getConfig
  2. trait implementing pt. 1
  3. Additional Interface with method getApigilityConfig + manual merge (e.g. ApigilityCustomConfigInterface)

I don't think it would be too much to ask the developer to do this since he is already following the tutorial on integrating Apigility into an existing module.

I would be happy to implement this if you think it is worthwhile in any way you seem fit.

Best regards,
Fran Pregernik


Originally posted by @FranPregernik at zfcampus/zf-apigility#69

@michalbundyra
Copy link
Member Author

@FranPregernik @weierophinney Is this looked at/thought about already?

This could be realy helpfull during development, we keep getting merge conflicts in the module.config.php using git.


Originally posted by @kingkorf at zfcampus/zf-apigility#69 (comment)

@michalbundyra
Copy link
Member Author

@kingkorf - Not that I am aware of.


Originally posted by @FranPregernik at zfcampus/zf-apigility#69 (comment)

@michalbundyra
Copy link
Member Author

@kingkorf I agree about this being a pain point with a versioning system. We have about 6 devs constantly adding services to an API and I have yet to see a clean merge of this one file fits all architecture.

Something should be done to make this set up more team friendly.


Originally posted by @jonrwads at zfcampus/zf-apigility#69 (comment)

@pmarko
Copy link

pmarko commented Apr 21, 2020

We just leave "module.config.php" for apigility and create our own config "custom.config.php" and in the Module.php we merge both configs.

 public function getConfig()
 {
        $config = new Config(include __DIR__ . '/../config/module.config.php');
        $config->merge(new Config(include __DIR__ . '/../config/custom.config.php'));
        return $config;
 }

All our configuration is in "custom" config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants