Simplify approach to allowing closures in manifold config

Upon review, the value() helper does not meet the project's needs. Below is the original code being referred to.

if(is_callable($value)){
    $call_response = $value();
    if($call_response){
        config([$key => $call_response]);
    }
}elseif(is_string($value) && config($value)){
    config([$key => config($value)]);
}

The suggestion was to clean this up by using Laravel's value() helper which will simply return the argument it was passed should it be a string, or return the results of the closure should it be a closure. However, as you can in the above function we need to handle the parameter differently depending on its type. For a closure we need to execute the function and then simply assign the results to $key as a config (config([$key => $call_response]);). But, if we receive a string we do not simply assign the string to $key as a config, we need to pass it to the config() helper to get an existing config to then assign that result to $key as a config (config([$key => config($value)]);). A subtle but important difference.

I went ahead and mocked up two possible interpretations of the suggestion, given that it did not come with examples itself.

$value = value($value);
if($value){
    config([$key => config($value)]);
}

As you can see, this does not work because if we receive a closure we are then passing the results of the closure to config(). As noted above, we only pass the parameter to config if the parameter is a string. Here's a second interpretation.

if(is_callable($value)){
    $value = value($value);
    if($value){
        config([$key => $value]);
    }
}elseif(is_string($value) && config($value)){
    config([$key => config($value)]);
}

This works fine but as you can see, hasn't really changed much from the original code. The problem is that our custom logic (when do we pass to config() to get a value to assign to $key and when to we just assign the $value directly to $key) needs to differentiate between a string and a closure.

The initial intent of the aliases feature was to allow a user to define the key of an existing configuration to assign to/overwrite an existing configuration. The use case would be something like my-project.MYSQL_PASSWORD, being a configuration pulled from Manifold, that needed to be used in your database.php config file. You could not simply use the config() helper in the database.php config file to get my-project.MYSQL_PASSWORD because configs aren't loaded yet at that point (it is in the middle of populating configurations). For this reason the aliases functionality exists, which will let a user define 'database.connections.mysql.password' => 'my-project.MYSQL_PASSWORD' in their manifold.php config.

The functionality was extended to accept a closure instead of just the key of an existing configuration. This was to allow users to manipulate results from Manifold, should they need to be parsed or interpreted. The use case that inspired this was how JAWS database configurations are currently stored/returned, which is as a single url-syntax-style connection string. This needs to be parsed by the user and each component set to a different configuration to play nice with Laravel's DB configurations without manipulating a bunch of boilerplate and/or core code. There is an example of this in the readme.

Suggesting this issue be closed with no code changes.

manifoldco / manifold-laravel

Simplify approach to allowing closures in manifold config #12