cakephp / cakephp/phinx

Parse config and replace values when instantiating

Open
#1,779 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
PHP
Stars
4.5k
Forks
884
PR merge metrics
No merged PRs in 30d

Description

The config object is parsed with values replaced in three different spots.

The first parsing is done when the Config class is instantiated and it replaces placeholders with environment variables:

https://github.com/cakephp/phinx/blob/dcc3e5135f7c093e8a53f90008a5266b87d69b17/src/Phinx/Config/Config.php#L51-L55

The second parsing is when getting a specific environment. This parses a DSN string if provided as well as deals with the special case of {"adapter": "sqlite", "memory": true}:

https://github.com/cakephp/phinx/blob/dcc3e5135f7c093e8a53f90008a5266b87d69b17/src/Phinx/Config/Config.php#L161-L183

The third parsing happens in Migration/Manager/Environment.php where it handles the special case of if connection key is set to PDO object (and handles throwing exception if adapter is not set):

https://github.com/cakephp/phinx/blob/dcc3e5135f7c093e8a53f90008a5266b87d69b17/src/Phinx/Migration/Manager/Environment.php#L336-L347

I think it would be good to condense all of this into one parse layer when the config object is initialized such that it:

  1. replaces environment variables
  2. parses DSN
  3. handles special case of sqlite + memory
  4. parses special case of connection

We could also make config throw an error as well if passed invalid environments as well, which would solve the problem of mixing validation and outputting as discussed in #1773. An upside of this is that now getEnvironments() and getEnvironment($environment) will now not return different things for a specific environment.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the referenced parsing sections in src/Phinx/Config/Config.php and src/Phinx/Migration/Manager/Environment.php, then compare what each currently returns for environment variables, DSNs, SQLite memory settings, and PDO connections. Done means parsing is consolidated during Config initialization and getEnvironments() and getEnvironment($environment) return consistent results; invalid environments may also be validated there.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
database
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.