Conversation
* Refactor: env * feat: add support for .env configuration files and dotenv integration * feat: add parameters configuration file creation to ScriptHandler * remove legacy public app files * fix: correct environment variable naming for parallel usage with phplist3 --------- Co-authored-by: Tatevik <tatevikg1@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces deployment defaults with dotenv configuration, adds configurable Doctrine table prefixes, and updates migrations, messaging configuration, domain behavior, documentation, and administrator defaults. ChangesEnvironment configuration migration
Database prefixing
Messaging and domain updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Composer
participant ScriptHandler
participant Bootstrap
participant Dotenv
participant Doctrine
Composer->>ScriptHandler: run update-configuration
ScriptHandler->>Dotenv: create /.env from /.env.dist
Bootstrap->>Dotenv: load and validate environment variables
Bootstrap->>Doctrine: configure application metadata
Doctrine->>Doctrine: apply DATABASE_PREFIX to eligible tables
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Composer/ScriptHandler.php (1)
283-285: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftHandle existing
.envfiles during upgrades.Preserving an existing file avoids overwriting deployment secrets, but it also prevents newly required keys from
.env.distfrom being added. Sinceconfig/parameters.ymlnow references those keys without inline fallbacks, older or partial.envfiles can leave required parameters unresolved. Add a merge/validation step or document an explicit upgrade procedure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Composer/ScriptHandler.php` around lines 283 - 285, Update the existing `.env` handling in the upgrade flow so an early return from the file-exists check no longer skips required-key validation and incorporation of newly introduced values from `.env.dist`. Preserve existing deployment secrets while merging only missing required keys, or invoke the project’s established validation/upgrade mechanism to ensure parameters referenced by `config/parameters.yml` are resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.dist:
- Around line 55-57: Update the environment-loading flow around
loadEnvironmentVariables so .env.dist remains template-only and PHPLIST_SECRET
never accepts the literal “%s” placeholder. Require a non-placeholder generated
or explicitly supplied secret, and fail fast when it is missing before security
tokens are used.
In `@CHANGELOG.md`:
- Line 13: Update the changelog entry to reference the current parameters
template path, config/parameters.yml, or explicitly state that
config/parameters.yml.dist was renamed to config/parameters.yml; keep the
description of defaults moving to .env.dist unchanged.
---
Nitpick comments:
In `@src/Composer/ScriptHandler.php`:
- Around line 283-285: Update the existing `.env` handling in the upgrade flow
so an early return from the file-exists check no longer skips required-key
validation and incorporation of newly introduced values from `.env.dist`.
Preserve existing deployment secrets while merging only missing required keys,
or invoke the project’s established validation/upgrade mechanism to ensure
parameters referenced by `config/parameters.yml` are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c64df25-bce9-461d-83d0-2f202a1f882d
📒 Files selected for processing (12)
.env.dist.gitignoreCHANGELOG.mdREADME.mdcomposer.jsonconfig/parameters.ymlconfig/parameters.yml.distpublic/app.phppublic/app_dev.phppublic/app_test.phpsrc/Composer/ScriptHandler.phpsrc/Core/Bootstrap.php
💤 Files with no reviewable changes (4)
- public/app_test.php
- config/parameters.yml.dist
- public/app_dev.php
- public/app.php
| - `symfony/dotenv` support: configuration values are now read from a `.env` file (generated from `.env.dist` on install/update), in addition to real environment variables (#TBD) | ||
|
|
||
| ### Changed | ||
| - `config/parameters.yml.dist` no longer contains inline `env(VAR): default` fallbacks; defaults now live in `.env.dist` (#TBD) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the current parameters template path.
This entry names config/parameters.yml.dist, while the migrated template and ScriptHandler now use config/parameters.yml. Use the current path or explicitly describe the rename.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 13, Update the changelog entry to reference the current
parameters template path, config/parameters.yml, or explicitly state that
config/parameters.yml.dist was renamed to config/parameters.yml; keep the
description of defaults moving to .env.dist unchanged.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.dist:
- Line 19: Replace the usable PHPLIST_ADMIN_PASSWORD value in the environment
template with a non-operational placeholder, and update the bootstrap logic
around the .env fallback (the code loading .env.dist near Bootstrap
initialization) to fail fast when that placeholder or an otherwise missing
deployment password is used, ensuring deployment must provide a strong
administrator password.
In `@src/Domain/Identity/Command/ImportDefaultsCommand.php`:
- Line 26: Update the default-user existence check in ImportDefaultsCommand to
recognize both the current DEFAULT_LOGIN value “test1” and the legacy “admin”
login before prompting or creating a user. Preserve the existing behavior for
fresh installations while preventing creation of a duplicate superuser when
either default account already exists.
- Line 26: Update the fallback prompt in ImportDefaultsCommand to interpolate
the `$login` value instead of hardcoding “admin”, so it reflects the
DEFAULT_LOGIN constant and any supplied login.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a67eb95-daed-4ef7-af7e-7f0513b09068
📒 Files selected for processing (3)
.env.distconfig/parameters.ymlsrc/Domain/Identity/Command/ImportDefaultsCommand.php
🚧 Files skipped from review as they are similar to previous changes (1)
- config/parameters.yml
| class ImportDefaultsCommand extends Command | ||
| { | ||
| private const DEFAULT_LOGIN = 'admin'; | ||
| private const DEFAULT_LOGIN = 'test1'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'phplist:defaults:import|ImportDefaultsCommand|DEFAULT_LOGIN|loginName|test1|admin' . || trueRepository: phpList/core
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
git ls-files | rg 'ImportDefaultsCommand|Administrator|default.*import|defaults|admin' | head -200
echo
echo "== ImportDefaultsCommand =="
if [ -f src/Domain/Identity/Command/ImportDefaultsCommand.php ]; then
ast-grep outline src/Domain/Identity/Command/999999999.php || true
cat -n src/Domain/Identity/Command/ImportDefaultsCommand.php
fi
echo
echo "== Targeted searches =="
rg -n 'DEFAULT_LOGIN|DEFAULT_PASSWORD|DEFAULT_EMAIL|test1|admin' src tests --glob '*.php' | head -120
rg -n 'phplist:defaults:import|defaults:import' . | head -120Repository: phpList/core
Length of output: 25665
Check legacy default users before creating test1.
phplist:defaults:import only looks for loginName = 'test1', so an existing installation with the previous default admin will prompt and create a second superuser with the default password. Treat admin as a legacy default or only run this command on fresh/empty admin tables.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Domain/Identity/Command/ImportDefaultsCommand.php` at line 26, Update the
default-user existence check in ImportDefaultsCommand to recognize both the
current DEFAULT_LOGIN value “test1” and the legacy “admin” login before
prompting or creating a user. Preserve the existing behavior for fresh
installations while preventing creation of a duplicate superuser when either
default account already exists.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the fallback prompt for the new login.
DEFAULT_LOGIN is now test1, but the prompt at Line 53 still says admin. Build the prompt from $login.
Proposed fix
- $question = new Question('Enter password for default admin (login "admin"): ');
+ $question = new Question(sprintf('Enter password for default admin (login "%s"): ', $login));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Domain/Identity/Command/ImportDefaultsCommand.php` at line 26, Update the
fallback prompt in ImportDefaultsCommand to interpolate the `$login` value
instead of hardcoding “admin”, so it reflects the DEFAULT_LOGIN constant and any
supplied login.
… TablePrefixListener for dynamic table prefixing
…namic table prefixing in migrations
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Core/Doctrine/TablePrefixListener.php`:
- Around line 30-32: Update TablePrefixListener so setPrimaryTable preserves and
prefixes metadata indexes and uniqueConstraints using the configured table
prefix, matching the renamed primary table and AbstractPrefixedMigration SQL
identifiers. Ensure index and unique-constraint names receive the prefix
consistently while retaining their existing definitions.
In `@src/Domain/Identity/Model/Administrator.php`:
- Line 28: Align the ORM metadata names with the prefixed migration names in
Administrator.php at line 28 and TemplateImage.php at line 13. Update the
relevant unique-constraint/index names using the DATABASE_PREFIX-aware naming
convention, or skip creating those explicitly named objects when a custom prefix
is active, so Doctrine schema tooling matches AbstractPrefixedMigration and
TablePrefixListener behavior.
In `@src/Domain/Messaging/Model/ListMessage.php`:
- Around line 17-19: Align explicit index and unique-constraint names with the
runtime table prefix, either by applying the TablePrefixListener naming pattern
or by making them database-default/unprefixed. Apply this consistently at
src/Domain/Messaging/Model/ListMessage.php:17-19, Message.php:25-26,
MessageAttachment.php:12-14, Template.php:15-16, UserMessage.php:15-20,
UserMessageBounce.php:14-18, UserMessageForward.php:14-17, and
src/Domain/Subscription/Model/SubscriberList.php:28-30; update the attributes in
each location so Doctrine’s metadata names follow the configured prefix.
In `@src/Migrations/AbstractPrefixedMigration.php`:
- Around line 17-27: Restrict prefix rewriting in
AbstractPrefixedMigration::addSql() to SQL schema identifiers rather than
applying str_replace across the entire SQL text; use explicit identifier
substitution or token-aware SQL rewriting while preserving parameters and types.
Apply this root-cause fix for the SQL consumed by
Version20251028092901MySqlInit::up(); no direct change is required in
src/Migrations/Version20251028092901MySqlInit.php:13.
In `@src/Migrations/Version20251028092902MySqlUpdate.php`:
- Line 11: Apply the custom DATABASE_PREFIX policy to both migration index names
and ORM index metadata so they remain consistent when the prefix differs from
phplist_. Update Version20251028092902MySqlUpdate.php and the index definitions
in UserMessageView.php, UserStats.php, Subscriber.php,
SubscriberAttributeDefinition.php, SubscriberAttributeValue.php,
SubscriberHistory.php, and Subscription.php at the specified ranges; use the
existing prefix-aware mechanism rather than hardcoded phplist_ names, and ensure
the migration update/down paths match the entity metadata.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 83ffa9fd-132b-4f22-9b41-db0b3536f4ff
📒 Files selected for processing (50)
config/services.ymlsrc/Core/Doctrine/TablePrefixListener.phpsrc/Domain/Analytics/Model/LinkTrack.phpsrc/Domain/Analytics/Model/LinkTrackForward.phpsrc/Domain/Analytics/Model/LinkTrackMl.phpsrc/Domain/Analytics/Model/LinkTrackUmlClick.phpsrc/Domain/Analytics/Model/LinkTrackUserClick.phpsrc/Domain/Analytics/Model/UserMessageView.phpsrc/Domain/Analytics/Model/UserStats.phpsrc/Domain/Configuration/Model/Config.phpsrc/Domain/Configuration/Model/EventLog.phpsrc/Domain/Configuration/Model/I18n.phpsrc/Domain/Configuration/Model/UrlCache.phpsrc/Domain/Identity/Model/AdminAttributeDefinition.phpsrc/Domain/Identity/Model/AdminAttributeValue.phpsrc/Domain/Identity/Model/AdminLogin.phpsrc/Domain/Identity/Model/AdminPasswordRequest.phpsrc/Domain/Identity/Model/Administrator.phpsrc/Domain/Identity/Model/AdministratorToken.phpsrc/Domain/Messaging/Model/Attachment.phpsrc/Domain/Messaging/Model/Bounce.phpsrc/Domain/Messaging/Model/BounceRegex.phpsrc/Domain/Messaging/Model/BounceRegexBounce.phpsrc/Domain/Messaging/Model/ListMessage.phpsrc/Domain/Messaging/Model/Message.phpsrc/Domain/Messaging/Model/MessageAttachment.phpsrc/Domain/Messaging/Model/MessageData.phpsrc/Domain/Messaging/Model/SendProcess.phpsrc/Domain/Messaging/Model/Template.phpsrc/Domain/Messaging/Model/TemplateImage.phpsrc/Domain/Messaging/Model/UserMessage.phpsrc/Domain/Messaging/Model/UserMessageBounce.phpsrc/Domain/Messaging/Model/UserMessageForward.phpsrc/Domain/Subscription/Model/SubscribePage.phpsrc/Domain/Subscription/Model/SubscribePageData.phpsrc/Domain/Subscription/Model/Subscriber.phpsrc/Domain/Subscription/Model/SubscriberAttributeDefinition.phpsrc/Domain/Subscription/Model/SubscriberAttributeValue.phpsrc/Domain/Subscription/Model/SubscriberHistory.phpsrc/Domain/Subscription/Model/SubscriberList.phpsrc/Domain/Subscription/Model/Subscription.phpsrc/Domain/Subscription/Model/UserBlacklist.phpsrc/Domain/Subscription/Model/UserBlacklistData.phpsrc/Migrations/AbstractPrefixedMigration.phpsrc/Migrations/Version20251028092901MySqlInit.phpsrc/Migrations/Version20251028092902MySqlUpdate.phpsrc/Migrations/Version20251031072945PostGreInit.phpsrc/Migrations/Version20260204094237.phpsrc/Migrations/_template_migration.php.tpltests/Unit/Domain/Configuration/Service/Provider/ConfigProviderTest.php
| $metadata->setPrimaryTable([ | ||
| 'name' => $this->tablePrefix . $metadata->getTableName(), | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '== Prefix implementation =='
rg -n -C 4 'str_replace|DATABASE_PREFIX|AbstractPrefixedMigration|setPrimaryTable' \
src/Migrations src/Core/Doctrine
echo
echo '== Static index and constraint names =='
rg -n -C 2 'ORM\\(Index|ORM\\(UniqueConstraint|phplist_' \
src/Domain --glob '*.php'Repository: phpList/core
Length of output: 6708
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '== Candidate files =='
git ls-files | rg '(^|/)(TablePrefixListener|AbstractPrefixedMigration)\.php$|composer\.lock$|composer\.json$' || true
echo
echo '== TablePrefixListener =="
sed -n '1,100p' src/Core/Doctrine/TablePrefixListener.php
echo
echo '== AbstractPrefixedMigration =="
sed -n '1,120p' src/Migrations/AbstractPrefixedMigration.php
echo
echo '== Static index/constraint annotations and phplist_ references in source =="
rg -n -C 2 'Index\(|UniqueConstraint\(|phplist_|index=|constraint=' src --glob '*.php' --exclude-dir=vendor || true
echo
echo '== Doctrine ORM source reference if available locally =="
rg -n -C 3 'function setPrimaryTable|primaryTable|indexes|uniqueConstraints' -g 'ClassMetadata.php' vendor || true
echo
echo '== Required composer versions =="
python3 - <<'PY'
import json, sys, pathlib
for path in [pathlib.Path(p) for p in [pathlib.Path('composer.lock'), pathlib.Path('composer.json')] if pathlib.Path(p).exists()]:
print(f'-- {path} --')
data=json.loads(path.read_text())
if path.name=='composer.lock':
packages=data.get('packages',[])+data.get('packages-dev',[])
for pkg in packages:
if pkg.get('name') in {'doctrine/orm','phpstan/phpstan'}:
print(pkg['name'], pkg.get('version'), pkg.get('source',{}).get('reference',''))
else:
print(data.get('require'), data.get('require-dev'))
PYRepository: phpList/core
Length of output: 647
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '== TablePrefixListener =='
sed -n '1,120p' src/Core/Doctrine/TablePrefixListener.php
echo
echo '== AbstractPrefixedMigration =='
sed -n '1,120p' src/Migrations/AbstractPrefixedMigration.php
echo
echo '== Static index/constraint annotations and phplist_ references in source =='
rg -n -C 2 'Index\(|UniqueConstraint\(|phplist_|index=|constraint=' src --glob '*.php' || true
echo
echo '== Doctrine ORM source reference if available locally =='
rg -n -C 3 'function setPrimaryTable|primaryTable|indexes|uniqueConstraints' -g 'ClassMetadata.php' vendor || true
echo
echo '== Required composer versions =='
python3 - <<'PY'
import json, pathlib
for path in [pathlib.Path('composer.lock'), pathlib.Path('composer.json')]:
if not path.exists():
continue
print(f'-- {path} --')
data=json.loads(path.read_text())
if path.name == 'composer.lock':
packages=data.get('packages',[])+data.get('packages-dev',[])
for pkg in packages:
if pkg.get('name') in {'doctrine/orm','phpstan/phpstan'}:
print(pkg['name'], pkg.get('version'), pkg.get('source',{}).get('reference',''))
else:
print(data.get('require'), data.get('require-dev'))
PYRepository: phpList/core
Length of output: 50369
🌐 Web query:
Doctrine ORM 3.3 ClassMetadata setPrimaryTable only name indexes uniqueConstraints metadata
💡 Result:
In Doctrine ORM, including version 3.3, the setPrimaryTable method in the ClassMetadata class is used to define the primary table mapping for an entity [1][2][3]. The method accepts an associative array as its argument [2]. You can use this array to specify the table name, as well as configure indexes and unique constraints for that table [2]. The supported structure for the array passed to setPrimaryTable is as follows [2]: name: A string specifying the table name. indexes: An array of index definitions. uniqueConstraints: An array of unique constraint definitions. Example usage: $metadata->setPrimaryTable([ 'name' => 'my_table_name', 'indexes' => [ ['name' => 'idx_example', 'columns' => ['column1', 'column2']] ], 'uniqueConstraints' => [ ['name' => 'uniq_example', 'columns' => ['column3']] ] ]); Note that if you only provide a subset of these keys, the existing values for the omitted keys in the ClassMetadata instance are preserved [2]. Additionally, in recent versions of Doctrine ORM, it is recommended to use setPrimaryTable(['name' =>...]) instead of the legacy setTableName method [4][2].
Citations:
- 1: https://www.doctrine-project.org/projects/doctrine-orm/en/3.6/reference/php-mapping.html
- 2: https://github.com/webmozart/doctrine-orm/blob/master/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
- 3: https://phpdox.net/demo/Symfony2/classes/Doctrine_ORM_Mapping_ClassMetadataInfo.xhtml
- 4: https://github.com/doctrine/orm/blob/85243a60b55c9e8042471c42d49871e59d438dd2/UPGRADE.md
Align index and unique-constraint names with the configured prefix.
TablePrefixListener only updates the primary table name. Doctrine ORM preserves omitted indexes and uniqueConstraints in setPrimaryTable(), while AbstractPrefixedMigration::addSql() replaces phplist_ globally in migration SQL. A custom DATABASE_PREFIX can therefore leave the database with tenant_xxx_midindex while Doctrine metadata still declares phplist_linktrack_midindex, causing schema diff/validation drift. Prefix the metadata index and unique-constraint names in the listener, or avoid global phplist_ replacement for SQL identifier names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Core/Doctrine/TablePrefixListener.php` around lines 30 - 32, Update
TablePrefixListener so setPrimaryTable preserves and prefixes metadata indexes
and uniqueConstraints using the configured table prefix, matching the renamed
primary table and AbstractPrefixedMigration SQL identifiers. Ensure index and
unique-constraint names receive the prefix consistently while retaining their
existing definitions.
Source: MCP tools
| */ | ||
| #[ORM\Entity(repositoryClass: AdministratorRepository::class)] | ||
| #[ORM\Table(name: 'phplist_admin')] | ||
| #[ORM\Table(name: 'admin')] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'src/(Domain|.*Migration.*|.*Listener.*)|phpunit|doctrine' | sed -n '1,200p'
echo "== target files =="
for f in src/Domain/Identity/Model/Administrator.php src/Domain/Messaging/Model/TemplateImage.php; do
if [ -f "$f" ]; then
echo "--- $f"
nl -ba "$f" | sed -n '1,220p'
fi
done
echo "== searches for prefixes/listeners/migrations/attributes =="
rg -n "TablePrefixListener|AbstractPrefixedMigration|primaryTable|Table\\(|indexes|options|id=|unique|index|phplist_" . -g '!vendor/**' -g '!node_modules/**' | sed -n '1,260p'Repository: phpList/core
Length of output: 11699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target relevant files =="
for f in src/Domain/Identity/Model/Administrator.php src/Domain/Messaging/Model/TemplateImage.php src/Core/Doctrine/TablePrefixListener.php; do
if [ -f "$f" ]; then
echo "--- $f"
awk '{printf "%3d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
done
echo "== migration files outline =="
fd -a 'migration|Migration' src . | sed -n '1,200p' | while IFS= read -r f; do
case "$f" in
*.php) echo "--- $f"; awk '{printf "%3d\t%s\n", NR, $0}' "$f" | sed -n '1,260p';;
esac
done
echo "== searches =="
rg -n "AbstractPrefixedMigration|TablePrefixListener|primaryTable|Table\\(|indexes|uniqueConstraints|Options|phplist_admin_loginnameidx|phplist_templateimage_templateidx|doctrine:generate|schema:generate|ORM\\:Table|ORM\\:Index|ORM\\:UniqueConstraint|ORM\\:JoinColumn" . --glob '*.php' --glob '*.yml' --glob '*.yaml' | sed -n '1,500p'Repository: phpList/core
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration contexts =="
for f in src/Migrations/Version20251028092901MySqlInit.php src/Migrations/Version20251028092902MySqlUpdate.php src/Migrations/Version20251031072945PostGreInit.php; do
echo "--- $f"
awk '{printf "%3d\t%s\n", NR, $0}' "$f" | sed -n '1,240p'
done
echo "== migrations that add/admin/template indexes around up/down =="
rg -n "admin_login|templateimage_template|CREATE INDEX|CREATE UNIQUE|DROP INDEX|RENAME INDEX|phplist_admin_loginnameidx|phplist_templateimage_templateidx" src/Migrations tests -g '*.php' | sed -n '1,260p'Repository: phpList/core
Length of output: 50369
🌐 Web query:
Doctrine ORM tablePrefix attribute unique constraint index name setPrimaryTable only table name
💡 Result:
There is no built-in "tablePrefix" attribute in Doctrine ORM [1][2]. To implement table prefixing, the recommended approach is to use a LoadClassMetadata listener that modifies the metadata at runtime [1][3]. For implementing a prefix via a listener: 1. You can hook into the loadClassMetadata event [1][3]. 2. Inside the listener, you use the ClassMetadata::setPrimaryTable method to prepend your prefix to the table name [1][4][3]. 3. Note that this listener must be registered before the EntityManager is initialized to ensure it is applied to all entities [1][3]. Regarding "setPrimaryTable" and table names: * setPrimaryTable is the correct method for updating the primary table configuration [4]. * It accepts an array where you can define the 'name' key to set the desired table name [1][3]. Regarding unique constraints and index names: * Doctrine ORM does not automatically apply table prefixes to index or unique constraint names defined in your entity configuration [5]. * If you need unique constraint or index names to also be prefixed, you must manually include the prefix in the 'name' attribute of the #[UniqueConstraint] or #[Index] attributes [5]. * Please note that defining uniqueConstraints and indexes inside the #[Table] attribute is deprecated and often ignored in modern Doctrine versions; it is recommended to define these as separate class-level attributes instead [6][7]. For a robust, automated solution that handles indexes, unique constraints, and sequences, community bundles such as DoctrinePrefixBundle are often used to avoid manual implementation [5].
Citations:
- 1: https://www.doctrine-project.org/projects/doctrine-orm/en/latest/cookbook/sql-table-prefixes.html
- 2: https://www.doctrine-project.org/projects/doctrine-orm/en/latest/reference/attributes-reference.html
- 3: https://www.doctrine-project.org/projects/doctrine-orm/en/3.6/cookbook/sql-table-prefixes.html
- 4: Update table prefix docs doctrine/orm#6695
- 5: https://github.com/roukmoute/DoctrinePrefixBundle/tree/master
- 6:
uniqueConstraintsare ignored in#[Table]doctrine/orm#10422 - 7:
AttributeDriverignores index and unique constraint definitions inTableattribute doctrine/orm#11351
🌐 Web query:
Doctrine ORM tablePrefix setPrimaryTable indexedColumns indexes uniqueConstraints
💡 Result:
In Doctrine ORM, configuration of table-level metadata such as indexes and unique constraints is handled through class-level attributes, annotations, or XML/YAML mapping [1][2][3]. Table Prefixes Doctrine does not provide a native configuration option for global table prefixes. The authoritative approach for implementing them is to use a loadClassMetadata event listener [4][5]. By registering this listener before the EntityManager is initialized, you can dynamically modify the class metadata to prepend a prefix to the table name and, if necessary, join table names [4][6]. Indexes and Unique Constraints Indexes and unique constraints are defined at the entity class level using the #[Index] and #[UniqueConstraint] attributes (or their annotation/XML equivalents) [1][2][7][3]. 1. Usage: You apply these attributes to the entity class [1][7]. They provide hints to the SchemaTool to generate the corresponding database structures [1][3]. 2. Configuration: - Indexes: Require a fields or columns definition and optionally accept a name and platform-specific options (e.g., where clause for partial indexes) [1][7]. - Unique Constraints: Require a fields or columns definition [1][3]. 3. Mapping Note: While some earlier documentation or implementations suggested nested configuration within the #[Table] attribute, current best practice and driver behavior emphasize defining these as independent class-level attributes [8]. Defining them directly on the class ensures they are correctly picked up by the attribute driver, avoiding potential issues where nested definitions are ignored [8]. Internal Metadata (setPrimaryTable) The setPrimaryTable method in ClassMetadata is used to define the primary table's metadata programmatically [9][10]. While primarily used internally, it is the mechanism employed by the table prefix listener to update the table name dynamically [4][5].
Citations:
- 1: https://www.doctrine-project.org/projects/doctrine-orm/en/3.6/reference/attributes-reference.html
- 2: https://www.doctrine-project.org/projects/doctrine-orm/en/2.17/reference/xml-mapping.html
- 3: https://www.doctrine-project.org/projects/doctrine-orm/en/2.21/reference/annotations-reference.html
- 4: https://www.doctrine-project.org/projects/doctrine-orm/en/latest/cookbook/sql-table-prefixes.html
- 5: https://www.doctrine-project.org/projects/doctrine-orm/en/3.6/cookbook/sql-table-prefixes.html
- 6: https://www.doctrine-project.org/projects/doctrine-orm/en/2.21/cookbook/sql-table-prefixes.html
- 7: https://www.doctrine-project.org/projects/doctrine-orm/en/latest/reference/attributes-reference.html
- 8:
AttributeDriverignores index and unique constraint definitions inTableattribute doctrine/orm#11351 - 9: https://www.doctrine-project.org/projects/doctrine-orm/en/3.6/reference/php-mapping.html
- 10: https://www.doctrine-project.org/projects/doctrine-orm/en/2.21/reference/php-mapping.html
Prefix the ORM meta-index/constraint names to match the SQL migration names.
TablePrefixListener adds the configured prefix only to the table name (primaryTable.name). AbstractPrefixedMigration::* will rename phplist_admin_loginnameidx/phplist_templateimage_templateidx according to migration SQL, but Doctrine schema tooling still sees #ADMINISTRATOR_UNIQUE_CONSTRAINT_NAME/#TEMPLATEIMAGE_INDEX_NAME. Use the same DATABASE_PREFIX-aware names here, or avoid creating those named objects in the custom-prefix path.
📍 Affects 2 files
src/Domain/Identity/Model/Administrator.php#L28-L28(this comment)src/Domain/Messaging/Model/TemplateImage.php#L13-L13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Domain/Identity/Model/Administrator.php` at line 28, Align the ORM
metadata names with the prefixed migration names in Administrator.php at line 28
and TemplateImage.php at line 13. Update the relevant unique-constraint/index
names using the DATABASE_PREFIX-aware naming convention, or skip creating those
explicitly named objects when a custom prefix is active, so Doctrine schema
tooling matches AbstractPrefixedMigration and TablePrefixListener behavior.
| #[ORM\Table(name: 'listmessage')] | ||
| #[ORM\UniqueConstraint(name: 'phplist_listmessage_messageid', columns: ['messageid', 'listid'])] | ||
| #[ORM\Index(name: 'phplist_listmessage_listmessageidx', columns: ['listid', 'messageid'])] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n 'phplist_|DATABASE_PREFIX|str_replace' \
src/Migrations/AbstractPrefixedMigration.php \
src/Migrations
rg -n 'phplist_(listmessage|message|message_attachment|template|usermessage|user_message_bounce|user_message_forward|list)_' \
src/Domain/Messaging/Model \
src/Domain/Subscription/ModelRepository: phpList/core
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AbstractPrefixedMigration =="
cat -n src/Migrations/AbstractPrefixedMigration.php
echo "== relevant model annotations =="
for f in \
src/Domain/Messaging/Model/ListMessage.php \
src/Domain/Messaging/Model/Message.php \
src/Domain/Messaging/Model/MessageAttachment.php \
src/Domain/Messaging/Model/Template.php \
src/Domain/Messaging/Model/UserMessage.php \
src/Domain/Messaging/Model/UserMessageBounce.php \
src/Domain/Messaging/Model/UserMessageForward.php \
src/Domain/Subscription/Model/SubscriberList.php
do
echo "--- $f"
sed -n '1,80p' "$f" | cat -n
done
echo "== migration versions mentioning these tables/indexes =="
rg -n "class Version|\\$this->addSql|RENAME INDEX|CREATE INDEX|CREATE TABLE|phplist_(listmessage|message|message_attachment|template|usermessage|user_message_bounce|user_message_forward|list)(_|$)" src/Migrations \
| awk '/class Version/ {print; flag=1} flag && /phplist_(listmessage|message|message_attachment|template|usermessage|user_message_bounce|user_message_forward|list)(_|$)/ {print} flag && /^[[:space:]]*};?$/ {flag=0}'
echo "== PHP files containing Doctrine prefix listener code =="
rg -n "TablePrefixLister|TablePrefixesSchemaManager|prefix|prefixes|PrefixesSchemaManager" src -g '*.php'Repository: phpList/core
Length of output: 24589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
indexes = [
("src/Domain/Messaging/Model/ListMessage.php", ["phplist_listmessage_messageid", "phplist_listmessage_listmessageidx"]),
("src/Domain/Messaging/Model/Message.php", ["phplist_message_uuididx"]),
("src/Domain/Messaging/Model/MessageAttachment.php", ["phplist_message_attachment_messageattidx", "phplist_message_attachment_messageidx"]),
("src/Domain/Messaging/Model/Template.php", ["phplist_template_title"]),
("src/Domain/Messaging/Model/UserMessage.php", ["phplist_usermessage_enteredindex", "phplist_usermessage_messageidindex", "phplist_usermessage_statusidx", "phplist_usermessage_useridindex", "phplist_usermessage_viewedidx"]),
("src/Domain/Messaging/Model/UserMessageBounce.php", ["phplist_user_message_bounce_bounceidx", "phplist_user_message_bounce_msgidx", "phplist_user_message_bounce_umbindex", "phplist_user_message_bounce_useridx"]),
("src/Domain/Messaging/Model/UserMessageForward.php", ["phplist_user_message_forward_messageidx", "phplist_user_message_forward_useridx", "phplist_user_message_forward_usermessageidx"]),
("src/Domain/Subscription/Model/SubscriberList.php", ["phplist_list_nameidx", "phplist_list_listorderidx"]),
]
class_mapping = {
"src/Domain/Messaging/Model/ListMessage.php": "listmessage",
"src/Domain/Messaging/Model/Message.php": "message",
"src/Domain/Messaging/Model/MessageAttachment.php": "message_attachment",
"src/Domain/Messaging/Model/Template.php": "template",
"src/Domain/Messaging/Model/UserMessage.php": "usermessage",
"src/Domain/Messaging/Model/UserMessageBounce.php": "user_message_bounce",
"src/Domain/Messaging/Model/UserMessageForward.php": "user_message_forward",
"src/Domain/Subscription/Model/SubscriberList.php": "list",
}
prefix = "phplist_"
non_default = "myapp_"
for path, names in indexes:
cls = class_mapping[path]
derived_non_default = [re.sub(r'^'+re.escape(prefix), non_default, n, count=1) for n in names]
print(f"{path}:{cls}")
for i, (n, nn) in enumerate(zip(names, derived_non_default), 1):
print(f" {i}. {n} -> {nn}")
for file in Path("src/Migrations/Version*.php").glob("*"):
text = file.read_text(errors="ignore")
hits = []
for name, _ in ((n,a) for _,names in indexes for n,a in zip(names, class_mapping.values()) for _ in names):
pass
full_names = [n for _,names in indexes for n in names]
for line in text.splitlines():
for n in full_names:
if n in line:
hits.append((file.name, line))
break
if hits:
print(f"--- {file.name}")
for h in hits[:8]:
print(f"{h[0]}:{h[1]}")
PYRepository: phpList/core
Length of output: 2068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "class (Doctrine.*Prefix|.*TablePrefix|PrefixesSchema|TablePrefixesSchema|TablePrefixLister|TablePrefixes.*Schema|PrefixSchemaManager|*Prefix)" src -g '*.php' || true
mig_files=$(find src/Migrations -maxdepth 1 \( -name 'Version*.php' -o -name 'AbstractPrefixedMigration.php' \) -printf '%f\n' | sort)
echo "== first migration matches around listed tables/indexes =="
python3 - <<'PY'
from pathlib import Path
import re
targets = {
"src/Domain/Messaging/Model/ListMessage.php": ["phplist_listmessage_messageid", "phplist_listmessage_listmessageidx"],
"src/Domain/Messaging/Model/Message.php": ["phplist_message_uuididx"],
"src/Domain/Messaging/Model/MessageAttachment.php": ["phplist_message_attachment_messageattidx", "phplist_message_attachment_messageidx"],
"src/Domain/Messaging/Model/Template.php": ["phplist_template_title"],
"src/Domain/Messaging/Model/UserMessage.php": ["phplist_usermessage_enteredindex", "phplist_usermessage_messageidindex", "phplist_usermessage_statusidx", "phplist_usermessage_useridindex", "phplist_usermessage_viewedidx"],
"src/Domain/Messaging/Model/UserMessageBounce.php": ["phplist_user_message_bounce_bounceidx", "phplist_user_message_bounce_msgidx", "phplist_user_message_bounce_umbindex", "phplist_user_message_bounce_useridx"],
"src/Domain/Messaging/Model/UserMessageForward.php": ["phplist_user_message_forward_messageidx", "phplist_user_message_forward_useridx", "phplist_user_message_forward_usermessageidx"],
"src/Domain/Subscription/Model/SubscriberList.php": ["phplist_list_nameidx", "phplist_list_listorderidx"],
}
names = {n for names in targets.values() for n in names}
paths = sorted(Path("src/Migrations").glob("Version*.php") | filter(lambda p: "Version20251028092902MySqlUpdate.php" not in str(p)))[-10:]
for path in paths:
text = path.read_text(errors="ignore")
if any(n in text for n in names):
print(f"--- {path.name}")
for i,line in enumerate(text.splitlines(),1):
if any(n in line for n in names):
print(f"{i:04d}: {line}")
PYRepository: phpList/core
Length of output: 690
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== prefix-related classes in src =="
rg -n "class .*Prefix|class .*Prefixes.*Schema|TablePrefixLister|TablePrefixesSchema|PrefixesSchemaManager|TablePrefix" src -g '*.php' || true
echo "== all ORM mapping files with explicit index/constraint names for listed entities =="
for f in \
src/Domain/Messaging/Model/ListMessage.php \
src/Domain/Messaging/Model/Message.php \
src/Domain/Messaging/Model/MessageAttachment.php \
src/Domain/Messaging/Model/Template.php \
src/Domain/Messaging/Model/UserMessage.php \
src/Domain/Messaging/Model/UserMessageBounce.php \
src/Domain/Messaging/Model/UserMessageForward.php \
src/Domain/Subscription/Model/SubscriberList.php
do
echo "--- $f"
rg -n "#\\[ORM\\.(Index|UniqueConstraint)\\(" "$f" && echo
doneRepository: phpList/core
Length of output: 3157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TablePrefixListener =="
cat -n src/Core/Doctrine/TablePrefixListener.php
echo "== ORM index/constraint annotations in listed files =="
for f in \
src/Domain/Messaging/Model/ListMessage.php \
src/Domain/Messaging/Model/Message.php \
src/Domain/Messaging/Model/MessageAttachment.php \
src/Domain/Messaging/Model/Template.php \
src/Domain/Messaging/Model/UserMessage.php \
src/Domain/Messaging/Model\UserMessageBounce.php \
src/Domain/Messaging/Model\UserMessageForward.php \
src/Domain/Subscription/Model/SubscriberList.php
do
echo "--- $f"
rg -n "#\\\\\\[ORM\\.(Index|UniqueConstraint)\\(" "$f" || true
done
echo "== migration matches for listed indexes =="
python3 - <<'PY'
from pathlib import Path
import re
files = sorted(Path("src/Migrations").glob("Version*.php"))
names = {
"phplist_listmessage_messageid",
"phplist_listmessage_listmessageidx",
"phplist_message_uuididx",
"phplist_message_attachment_messageattidx",
"phplist_message_attachment_messageidx",
"phplist_template_title",
"phplist_usermessage_enteredindex",
"phplist_usermessage_messageidindex",
"phplist_usermessage_statusidx",
"phplist_usermessage_useridindex",
"phplist_usermessage_viewedidx",
"phplist_user_message_bounce_bounceidx",
"phplist_user_message_bounce_msgidx",
"phplist_user_message_bounce_umbindex",
"phplist_user_message_bounce_useridx",
"phplist_user_message_forward_messageidx",
"phplist_user_message_forward_useridx",
"phplist_user_message_forward_usermessageidx",
"phplist_list_nameidx",
"phplist_list_listorderidx",
}
for path in files:
text = path.read_text(errors="ignore")
hits = [(i, line.strip()) for i, line in enumerate(text.splitlines(),1) if any(n in line for n in names)]
if hits:
print(f"--- {path.name}")
for i,line in hits:
print(f"{i}: {line}")
print()
PYRepository: phpList/core
Length of output: 9048
Align entity index and constraint names with the table prefix.
TablePrefixListener changes the primary table name at runtime, but explicit #[ORM\Index(..., name: 'phplist_*')] and #[ORM\UniqueConstraint(..., name: 'phplist_*')] names do not change. This can make Doctrine detect changed indexes/constraints with non-default DATABASE_PREFIX. Apply the same listener pattern to explicit index/constraint names, or keep these names database-default/unprefixed.
📍 Affects 8 files
src/Domain/Messaging/Model/ListMessage.php#L17-L19(this comment)src/Domain/Messaging/Model/Message.php#L25-L26src/Domain/Messaging/Model/MessageAttachment.php#L12-L14src/Domain/Messaging/Model/Template.php#L15-L16src/Domain/Messaging/Model/UserMessage.php#L15-L20src/Domain/Messaging/Model/UserMessageBounce.php#L14-L18src/Domain/Messaging/Model/UserMessageForward.php#L14-L17src/Domain/Subscription/Model/SubscriberList.php#L28-L30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Domain/Messaging/Model/ListMessage.php` around lines 17 - 19, Align
explicit index and unique-constraint names with the runtime table prefix, either
by applying the TablePrefixListener naming pattern or by making them
database-default/unprefixed. Apply this consistently at
src/Domain/Messaging/Model/ListMessage.php:17-19, Message.php:25-26,
MessageAttachment.php:12-14, Template.php:15-16, UserMessage.php:15-20,
UserMessageBounce.php:14-18, UserMessageForward.php:14-17, and
src/Domain/Subscription/Model/SubscriberList.php:28-30; update the attributes in
each location so Doctrine’s metadata names follow the configured prefix.
Source: MCP tools
| protected function addSql(string $sql, array $params = [], array $types = []): void | ||
| { | ||
| parent::addSql( | ||
| str_replace( | ||
| self::DEFAULT_PREFIX, | ||
| $this->getTablePrefix(), | ||
| $sql | ||
| ), | ||
| $params, | ||
| $types | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)AbstractPrefixedMigration\.php$|(^|/)Version20251028092901MySqlInit\.php$|initial_schema\.sql$' || true
echo
echo "AbstractPrefixedMigration.php outline:"
ast-grep outline src/Migrations/AbstractPrefixedMigration.php --view expanded || true
echo
echo "AbstractPrefixedMigration.php:"
cat -n src/Migrations/AbstractPrefixedMigration.php
echo
echo "Version20251028092901MySqlInit.php:"
cat -n src/Migrations/Version20251028092901MySqlInit.php
echo
echo "initial_schema.sql lines count and sample:"
wc -l src/Migrations/initial_schema.sql || true
sed -n '1,220p' src/Migrations/initial_schema.sql | nl -ba
echo
echo "grep for phplist_ and DEFAULT_PREFIX refs in Migrations:"
rg -n "DEFAULT_PREFIX|phplist_|Version20251028092901MySqlInit|initial_schema.sql|addSql" src/Migrations || true
echo
echo "Read-only probe: demonstrate PHP-like full-text replacement behavior for non-identifier tokens"
python3 - <<'PY'
sql = "INSERT INTO users (name) VALUES ('prefix_phplist_abc');"
prefix = "phplist_"
print(sql.replace(prefix, ""))
PYRepository: phpList/core
Length of output: 3628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "initial_schema.sql occurrences:"
rg -n "phplist_" src/Migrations/initial_schema.sql | head -n 120 || true
echo
echo "initial_schema.sql total matches:"
rg -c "phplist_" src/Migrations/initial_schema.sql || true
echo
echo "initial_schema.sql line count:"
wc -l src/Migrations/initial_schema.sql || true
echo
echo "initial_schema.sql sample around occurrences:"
python3 - <<'PY'
from pathlib import Path
lines = Path('src/Migrations/initial_schema.sql').read_text().splitlines()
matches = {i for i,l in enumerate(lines,1) if 'phplist_' in l}
for i in sorted(matches)[:40]:
lo = max(1, i-3); hi = min(len(lines), i+3)
print(f"\n--- lines {lo}-{hi} ---")
for n in range(lo, hi+1):
print(f"{n:4}: " + lines[n-1])
PY
echo
echo "Behavioral probe: full-text replacement matches non-identifier tokens"
python3 - <<'PY'
sql = """
-- phplist legacy comment
INSERT INTO users (name) VALUES ('prefix_phplist_abc');
SELECT 'phplist_value';
"""
print(sql.replace('phplist_', ''))
PYRepository: phpList/core
Length of output: 16111
Scope DATABASE_PREFIX replacement to schema identifiers.
AbstractPrefixedMigration::addSql() replaces phplist_ across the full SQL text, so Version20251028092901MySqlInit::up() can unintentionally change comments or any non-identifier phplist_ token in initial_schema.sql. Use explicit identifier rewriting or token-aware SQL rewriting instead.
[data_Integrity_and_integration]
📍 Affects 2 files
src/Migrations/AbstractPrefixedMigration.php#L17-L27(this comment)src/Migrations/Version20251028092901MySqlInit.php#L13-L13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Migrations/AbstractPrefixedMigration.php` around lines 17 - 27, Restrict
prefix rewriting in AbstractPrefixedMigration::addSql() to SQL schema
identifiers rather than applying str_replace across the entire SQL text; use
explicit identifier substitution or token-aware SQL rewriting while preserving
parameters and types. Apply this root-cause fix for the SQL consumed by
Version20251028092901MySqlInit::up(); no direct change is required in
src/Migrations/Version20251028092901MySqlInit.php:13.
| use Doctrine\DBAL\Schema\Schema; | ||
|
|
||
| final class Version20251028092902MySqlUpdate extends AbstractMigration | ||
| final class Version20251028092902MySqlUpdate extends AbstractPrefixedMigration |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -a 'Version20251028|AbstractPrefixedMigration|TablePrefixListener|.*\.php$' . | head -200
echo
echo "Target files existence and sizes:"
for f in \
src/Migrations/Version20251028092902MySqlUpdate.php \
src/Migrations/AbstractPrefixedMigration.php \
src/Migrations/Version20251028092901MySqlInit.php \
src/Domain/Analytics/Model/UserMessageView.php \
src/Domain/Analytics/Model/UserStats.php \
src/Domain/Subscription/Model/Subscriber.php \
src/Domain/Subscription/Model/SubscriberAttributeDefinition.php \
src/Domain/Subscription/Model/SubscriberAttributeValue.php \
src/Domain/Subscription/Model/SubscriberHistory.php \
src/Domain/Subscription/Model/Subscription.php
do
if [ -f "$f" ]; then wc -l "$f"; else echo "MISSING $f"; fi
doneRepository: phpList/core
Length of output: 14861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Specific target files:"
for f in \
src/Migrations/Version20251028092902MySqlUpdate.php \
src/Migrations/AbstractPrefixedMigration.php \
src/Migrations/Version20251028092901MySqlInit.php \
src/Core/Doctrine/TablePrefixListener.php \
src/Domain/Analytics/Model/UserMessageView.php \
src/Domain/Analytics/Model/UserStats.php \
src/Domain/Subscription/Model/Subscriber.php \
src/Domain/Subscription/Model/SubscriberAttributeDefinition.php \
src/Domain/Subscription/Model/SubscriberAttributeValue.php \
src/Domain/Subscription/Model/SubscriberHistory.php \
src/Domain/Subscription/Model/Subscription.php
do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
cat -n "$f"
else
echo "MISSING $f"
fi
doneRepository: phpList/core
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Listener and env wiring:"
rg -n "TablePrefixListener|DATABASE_PREFIX|tablePrefix|loadClassMetadata" src -g '*.php'
echo
echo "Entity metadata annotations (target files, relevant attrs only):"
python3 - <<'PY'
import re, pathlib
for path in [
pathlib.Path("src/Domain/Analytics/Model/UserMessageView.php"),
pathlib.Path("src/Domain/Analytics/Model/UserStats.php"),
pathlib.Path("src/Domain/Subscription/Model/Subscriber.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeDefinition.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeValue.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberHistory.php"),
pathlib.Path("src/Domain/Subscription/Model/Subscription.php")]:
text=pathlib.Path(path).read_text()
if path.name=="Subscriber.php":
lines=text.splitlines()
for i,line in enumerate(lines[:50],1):
if ("ORM\Table" in line or "ORM\Index" in line or "ORM\UniqueConstraint" in line or "ORM\JoinColumn" in line or "ORM\JoinTable" in line) and ("name:" in line):
print(f"{path}:{i}:{line}")
continue
for i,line in enumerate(text.splitlines(),1):
if ("ORM\Table" in line or "ORM\Index" in line or "ORM\UniqueConstraint" in line or "ORM\JoinColumn" in line or "ORM\JoinTable" in line) and ("name:" in line):
print(f"{path}:{i}:{line}")
PY
echo
echo "Migration update SQL names with prefix (TargetUpdate.php first 165 lines):"
python3 - <<'PY'
import subprocess, pathlib, re
text = pathlib.Path("src/Migrations/Version20251028092902MySqlUpdate.php").read_text()
m = re.search(r"public function up\(Schema \$schema\): void\s*{(.*?)\n public function down", text, re.S)
up = m.group(1) if m else ""
hits = []
for i,line in enumerate(up.splitlines(), 1):
if re.search(r"[A-Za-z_][A-Za-z0-9_]*(?:idx|[Ff][Kk]|PK|idx|[Uu][Nn][Ii][Qq](?:[Ii]D)?|[Ss]tatus|[Ee]ntry|[Dd]ateindex|[Ll]istindex|[Ii]temindex)|\bINDEX\b", line) and ("phplist" in line or "IDX_" in line or "FK_" in line or "foreignkey" in line.lower() or "`" in line):
hits.append((i,line))
print(f"{len(hits)} named schema-object occurrences")
prefix_re = re.compile(r"phplist_|^idx|uniqueindex|_?[A-Za-z]\w*idx|[Ff]oreign[A-Za-z]*key|[Uu]nique[A-Za-z0-9]*")
for i,line in hits[:80]:
print(f"{i}: {line.strip()}")
PY
echo
echo "Search for hardcoded unprefixed names in target entity files:"
python3 - <<'PY'
import pathlib, re
for path in [
pathlib.Path("src/Domain/Subscription/Model/Subscriber.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeDefinition.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeValue.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberHistory.php"),
pathlib.Path("src/Domain/Subscription/Model/Subscription.php")]:
text=pathlib.Path(path).read_text()
# print ORM Table/Index/Unique/ForeignKey line numbers where name field is present
for i,line in enumerate(text.splitlines(),1):
if 'name:' in line and any(s in line for s in ("ORM\\Table", "ORM\\Index", "ORM\\UniqueConstraint", "ORM\\JoinColumn", "ORM\\JoinTable")):
print(f"{path}:{i}:{line.strip()}")
PYRepository: phpList/core
Length of output: 1276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Dependency versions/config snippets:"
for f in composer.json composer.lock config/* dbconfig.php app/config/services.php src/Core/DoctrineMappingPass.php; do
[ -f "$f" ] && { echo "--- $f"; if [ "$f" = "composer.json" ]; then jq '.require' "$f"; else sed -n '1,220p' "$f"; fi; }
done
echo
echo "Target entity named schema mappings (selected attrs only):"
python3 - <<'PY'
import pathlib
paths = [
pathlib.Path("src/Domain/Subscription/Model/Subscriber.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeDefinition.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberAttributeValue.php"),
pathlib.Path("src/Domain/Subscription/Model/SubscriberHistory.php"),
pathlib.Path("src/Domain/Subscription/Model/Subscription.php")
]
for path in paths:
text = path.read_text()
print(f"--- {path}")
for i, line in enumerate(text.splitlines(), 1):
if "name:" in line and any(s in line for s in ("ORM\\Table", "ORM\\Index", "ORM\\UniqueConstraint", "ORM\\JoinColumn", "ORM\\JoinTable")):
print(f"{i}: {line.strip()}")
PY
echo
echo "Behavioral code-property probe: parse fixed migration SQL up statements into schema names and compare with hardcoded ORM names in target files."
python3 - <<'PY'
import pathlib, re
def schema_objs(sql: str):
# Capture names without applying prefix rewrite, only for token-level insight
names = []
for m in re.finditer(r'''# Rename/drop/alter/add references: CREATE INDEX|REN *?*|DROP INDEX|ADD CONSTRAINT|CREATE INDEX|CREATE UNIQUE INDEX|FOREIGN KEY|INDEX|CONSTR''' ...): pass
return names
update = pathlib.Path("src/Migrations/Version20251028092902MySqlUpdate.php").read_text()
main = re.search(r"public function up\\$schema\\): void\\s*{(.*?)}\\s*public function down", update, re.S).group(1)
print("Migration up line count:", len(main.splitlines()))
for i, line in enumerate(main.splitlines(), 1):
if "rename" in line.lower() or "renameindex" in line.lower() or "create index" in line.lower() or "add constraint" in line.lower() or "drop index" in line.lower() or "foreignkey" in line.lower():
if "phplist" in line:
print(f"{i}: {line.strip()}")
PYRepository: phpList/core
Length of output: 19370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Doctrine/onlyorm TablePrefixListener usages:"
rg -n "TablePrefixListener|OnlyOrmTablesFilter|loadClassMetadata|database_prefix|PHPLIST_DATABASE_PREFIX|DATABASE_PREFIX" src config -g '*.php' -g '*.yml' -g '*.yaml' -g '*.xml'
echo
echo "SQL named object extraction from target migration:"
python3 - <<'PY'
import pathlib, re
text = pathlib.Path('src/Migrations/Version20251028092902MySqlUpdate.php').read_text()
main = re.search(r'public function up\(Schema \\\$schema\): void\s*{(.*?)\n public function down', text, re.S).group(1)
for i, line in enumerate(main.splitlines(), 1):
line = line.strip()
sql = re.sub(r'#.*', '', line)
if any(name in line.lower() or 'index' in line.lower() or 'constraint' in line.lower() or 'foreign' in line.lower() for name in [
'phplist_', 'idx_', 'fk_', 'unique', 'key'
]):
print(f'{i}: {sql}')
PY
echo
echo "Read-only static summary of target entity/schema name policy:"
python3 - <<'PY'
import pathlib, re
target = [
'src/Domain/Analytics/Model/UserMessageView.php',
'src/Domain/Analytics/Model/UserStats.php',
'src/Domain/Subscription/Model/Subscriber.php',
'src/Domain/Subscription/Model/SubscriberAttributeDefinition.php',
'src/Domain/Subscription/Model/SubscriberAttributeValue.php',
'src/Domain/Subscription/Model/SubscriberHistory.php',
'src/Domain/Subscription/Model/Subscription.php'
]
for path in target:
lines = pathlib.Path(path).read_text().splitlines()
print(f'--- {path}')
for i, line in enumerate(lines, 1):
if any(n in line for n in ["ORM\\Table", "ORM\\Index", "ORM\\UniqueConstraint", 'ORM\\JoinColumn', "ORM\\JoinTable"]) and 'name:' in line:
print(f'{i}: {line.strip()}')
PYRepository: phpList/core
Length of output: 1293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target migration SQL named schema objects:"
python3 - <<'PY'
from pathlib import Path
text = Path('src/Migrations/Version20251028092902MySqlUpdate.php').read_text()
# Simple extraction for occurrences in lines that name indexes/constraints/aliases.
for i, line in enumerate(text.splitlines(), 1):
sql = line.split('addSql', 1)[-1] if 'addSql' in line else ''
if any(part in sql for part in ['phplist_', 'IDX_', 'FK_', 'unique', 'foreignkey', 'msgidx', 'useridx', 'usermsgidx', 'dateindex', 'listindex', 'itemindex', 'email', 'foreignkey', 'uuididx']):
print(f'{i}: {line.strip()}')
PY
echo
echo "Existing migration down renamed index examples for target entity:"
rg -n "phplist_user_(message_view|stats)|subscribpage|user_message_view|userstats|subscription|subscribpage|message_view|usermsgidx|dateindex|listindex|itemindex" src/Migrations src/Core src/Domain -g '*.php' -g '*.yml' -g '*.yaml' | head -200Repository: phpList/core
Length of output: 50368
Make the custom prefix policy apply consistently.
AbstractPrefixedMigration replaces phplist_ in tables and indexes, but TablePrefixListener only prefixes the ORM table name. Run Doctrine schema validation with DATABASE_PREFIX set to something other than phplist_; the update/down path assumes the index names in Version20251028092902MySqlUpdate.php, while the listed entity metadata uses the same hardcoded names.
📍 Affects 8 files
src/Migrations/Version20251028092902MySqlUpdate.php#L11-L11(this comment)src/Domain/Analytics/Model/UserMessageView.php#L14-L14src/Domain/Analytics/Model/UserStats.php#L13-L13src/Domain/Subscription/Model/Subscriber.php#L27-L27src/Domain/Subscription/Model/SubscriberAttributeDefinition.php#L15-L15src/Domain/Subscription/Model/SubscriberAttributeValue.php#L12-L12src/Domain/Subscription/Model/SubscriberHistory.php#L14-L14src/Domain/Subscription/Model/Subscription.php#L25-L25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Migrations/Version20251028092902MySqlUpdate.php` at line 11, Apply the
custom DATABASE_PREFIX policy to both migration index names and ORM index
metadata so they remain consistent when the prefix differs from phplist_. Update
Version20251028092902MySqlUpdate.php and the index definitions in
UserMessageView.php, UserStats.php, Subscriber.php,
SubscriberAttributeDefinition.php, SubscriberAttributeValue.php,
SubscriberHistory.php, and Subscription.php at the specified ranges; use the
existing prefix-aware mechanism rather than hardcoded phplist_ names, and ensure
the migration update/down paths match the entity metadata.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/Domain/Identity/Command/ImportDefaultsCommand.php (3)
43-49: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject whitespace-only configured passwords.
$this->defaultAdminPassword !== ''accepts values such as' 'and skips the prompt. The interactive path rejects the same value withtrim($password) === ''. Check the trimmed value before selecting the configured password, while preserving the original non-empty password.Proposed fix
- $password = $this->defaultAdminPassword !== '' ? $this->defaultAdminPassword : null; + $password = trim($this->defaultAdminPassword) !== '' + ? $this->defaultAdminPassword + : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Domain/Identity/Command/ImportDefaultsCommand.php` around lines 43 - 49, Update the password selection in ImportDefaultsCommand so whitespace-only defaultAdminPassword values are treated as unset and trigger the existing prompt path. Check the trimmed configured value for emptiness, while preserving the original untrimmed password when it contains non-whitespace characters.
33-34: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake
PHPLIST_DEFAULT_ADMIN_PASSWORDoptional or defaulted.
config/parameters.ymlrequiresPHPLIST_DEFAULT_ADMIN_PASSWORD, and.env.distdoes not define it. Symfony loads undefined%env(...)%values as failing beforeImportDefaultsCommandcan use the existing empty-string fallback, so an omitted variable can stop command startup. Add the variable to.env.distwith the intended fallback, or make the binding/property explicitly nullable/defaulted and wire that value into the prompt fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Domain/Identity/Command/ImportDefaultsCommand.php` around lines 33 - 34, Make the default admin password configuration optional by defining PHPLIST_DEFAULT_ADMIN_PASSWORD with its intended fallback in .env.dist, or by making the Autowire binding used by ImportDefaultsCommand explicitly nullable/defaulted. Ensure ImportDefaultsCommand continues using an empty value as the prompt fallback when the environment variable is omitted.
26-26: 🔒 Security & Privacy | 🟠 MajorPreserve the previous default login during upgrades.
When an existing installation still has the previous default user
test1, theadminlookup does not find it. The command can then create a second superuser with the default credential. Check bothadminandtest1before creating the account, or restrict this command to fresh installations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Domain/Identity/Command/ImportDefaultsCommand.php` at line 26, Update ImportDefaultsCommand to check for both the current DEFAULT_LOGIN value admin and the legacy login test1 before creating the default superuser, preventing duplicate accounts during upgrades while preserving creation for installations with neither login.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Core/Bootstrap.php`:
- Line 181: Update configureApplicationKernel around Dotenv::loadEnv() to read
the resolved APP_ENV after dotenv loading, validate that it is a supported
environment, and assign it to $this->environment before creating the kernel.
Ensure the kernel receives this resolved value so environment-specific files and
runtime configuration remain aligned.
---
Outside diff comments:
In `@src/Domain/Identity/Command/ImportDefaultsCommand.php`:
- Around line 43-49: Update the password selection in ImportDefaultsCommand so
whitespace-only defaultAdminPassword values are treated as unset and trigger the
existing prompt path. Check the trimmed configured value for emptiness, while
preserving the original untrimmed password when it contains non-whitespace
characters.
- Around line 33-34: Make the default admin password configuration optional by
defining PHPLIST_DEFAULT_ADMIN_PASSWORD with its intended fallback in .env.dist,
or by making the Autowire binding used by ImportDefaultsCommand explicitly
nullable/defaulted. Ensure ImportDefaultsCommand continues using an empty value
as the prompt fallback when the environment variable is omitted.
- Line 26: Update ImportDefaultsCommand to check for both the current
DEFAULT_LOGIN value admin and the legacy login test1 before creating the default
superuser, preventing duplicate accounts during upgrades while preserving
creation for installations with neither login.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eb0a38c7-772a-4869-b149-f9819c1bed63
📒 Files selected for processing (3)
src/Core/Bootstrap.phpsrc/Core/Doctrine/TablePrefixListener.phpsrc/Domain/Identity/Command/ImportDefaultsCommand.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Core/Doctrine/TablePrefixListener.php
| ); | ||
| } | ||
|
|
||
| (new Dotenv())->loadEnv($dotenvPath, 'APP_ENV', $this->environment); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate relevant files"
fd -a 'Bootstrap.php|ApplicationKernel.php|Environment.php' . | sed 's#^\./##' | head -50
echo
echo "Search Dotenv/APP_ENV usages"
rg -n "loadEnv|APP_ENV|configureApplicationKernel|\$environment|ApplicationKernel|class Environment" src -S || true
echo
echo "Inspect Bootstrap outline/size"
for f in $(fd 'Bootstrap.php' .); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view expanded || true
doneRepository: phpList/core
Length of output: 3189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Read target Bootstrap around loadEnv/kernel creation"
for f in $(fd 'Bootstrap.php' .); do
echo "--- $f"
nl -ba "$f" | sed -n '140,210p'
done
echo
echo "Read ApplicationKernel and Environment relevant files"
for f in $(fd 'ApplicationKernel.php|Environment.php' .); do
echo "--- $f ($(wc -l < "$f") lines)"
nl -ba "$f" | sed -n '1,260p'
done
echo
echo "Check Symfony Dotenv dependency declaration"
for f in composer.json composer.lock; do
[ -f "$f" ] && { echo "--- $f"; rg -n '"symfony/dotenv"|symfony/dotenv|version|name' "$f" -A 4 -B 4 || true; }
doneRepository: phpList/core
Length of output: 272
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Bootstrap lines 160-240"
awk 'NR>=160 && NR<=245 { printf "%6d\t%s\n", NR, $0 }' src/Core/Bootstrap.php
echo
echo "Environment class"
awk '{ printf "%6d\t%s\n", NR, $0 }' src/Core/Environment.php
echo
echo "ApplicationKernel class"
awk '{ printf "%6d\t%s\n", NR, $0 }' src/Core/ApplicationKernel.php
echo
echo "Dotenv references"
for f in composer.json composer.lock; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n '"symfony/dotenv"|symfony/dotenv|version' "$f" || true
fi
doneRepository: phpList/core
Length of output: 12253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Bootstrap public API/constructor/init around environments"
awk 'NR>=30 && NR<=165 { printf "%6d\t%s\n", NR, $0 }' src/Core/Bootstrap.php
echo
echo "Bootstrap exact configureApplicationKernel"
awk 'NR>=240 && NR<=260 { printf "%6d\t%s\n", NR, $0 }' src/Core/Bootstrap.php
echo
echo "Composer lock dotenv exact"
python3 - <<'PY'
import json, pathlib
p=pathlib.Path('composer.lock')
if not p.exists():
raise SystemExit
data=json.loads(p.read_text())
for pkg in data.get('packages', []):
if pkg.get('name') == 'symfony/dotenv':
print({k:c for k,c in pkg.items() if k in ('name','version','type','source','dist')})
PYRepository: phpList/core
Length of output: 5964
🌐 Web query:
Symfony Dotenv loadEnv function sets default value if already defined
💡 Result:
In Symfony's Dotenv component, the default behavior of the loadEnv method is to not overwrite existing environment variables [1][2][3]. When loadEnv parses.env files, it only populates $_SERVER or $_ENV for variables that are not already defined in the system's environment [4][5][2]. If you need to force Symfony to overwrite existing environment variables with values from your.env files, you can use the overrideExistingVars parameter [6][1]. This parameter is available in the loadEnv, bootEnv, and populate methods [1]. To enable this, set the parameter to true in your code: $dotenv = new Dotenv; $dotenv->loadEnv(DIR.'/.env', overrideExistingVars: true); This approach ensures that variables defined in your.env files take precedence over system-defined environment variables, while still respecting the hierarchy of the.env files themselves (e.g.,.env.local overriding.env) [1][7].
Citations:
- 1: https://symfony.com/doc/current/configuration.html
- 2: https://symfony.com/doc/3.x/components/dotenv.html
- 3: https://symfony.com/blog/improvements-to-the-handling-of-env-files-for-all-symfony-versions
- 4: https://github.com/symfony/dotenv
- 5: https://github.com/symfony/dotenv/tree/8.0
- 6: https://github.com/symfony/dotenv/blob/8.1/CHANGELOG.md
- 7: [Dotenv] add loadEnv(), a smoother alternative to loadForEnv() symfony/symfony#29129
🌐 Web query:
symfony/dotenv 6.4 loadEnv signature default env argument overrides existing vars false
💡 Result:
Symfony\Component\Dotenv\Dotenv::loadEnv signature (Symfony/dotenv v6.4) is: public function loadEnv(string $path,?string $envKey = null, string $defaultEnv = 'dev', array $testEnvs = ['test'], bool $overrideExistingVars = false): void [1] Default env argument behavior: - The $defaultEnv parameter defaults to 'dev' and is used when the app-env key ($envKey or the component’s configured envKey) is not already set in $_SERVER or $_ENV. In that case, Dotenv populates that env key to $defaultEnv before loading the env-specific files. [2] Does overrideExistingVars=false override existing variables? - The parameter $overrideExistingVars defaults to false and is described as: “Whether existing environment variables set by system should be overridden”. [1] - Symfony docs also show that when you set overrideExistingVars: true you override environment variables defined by the system, implying the default (false) does not override system env vars. [3] - However, real-world behavior can be subtle: there is an open GitHub issue reporting that variables can appear overridden “even when override is disabled” when loading multiple dotenv files in a particular order (e.g.,.env.local vs.env). This indicates that “false” applies to overriding existing values, but file-loading priority/sequence can still lead to surprising results depending on what was already present and the loading order. [4] So in short: with overrideExistingVars=false (the default), Dotenv should not override existing env vars set by the system, and $defaultEnv defaults to 'dev' when APP_ENV (or your configured env key) isn’t set; but multiple-dotenv loading order can still produce unexpected effective values. [1][3][4]
Citations:
- 1: https://github.com/symfony/symfony/blob/8.0/src/Symfony/Component/Dotenv/Dotenv.php
- 2: https://github.com/symfony/dotenv/blob/783f12027c6b40ab0e93d6136d9f642d1d67cd6b/Dotenv.php
- 3: https://symfony.com/doc/current/configuration.html
- 4: env variable is being overriden even when override is disabled symfony/symfony#52073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check whether overrideExistingVar path exists in lockfile/vendor-like refs"
python3 - <<'PY'
from pathlib import Path
p = Path('composer.lock')
if p.exists():
import json
data=json.loads(p.read_text())
for target in ('packages','packages-dev'):
for pkg in data.get(target, []):
if pkg.get('name') == 'symfony/dotenv':
print(json.dumps({k: v for k, v in pkg.items() if k in ('name','version','lock-version','source','dist','require','autoload')}, indent=2))
PY
git ls-files | rg 'symfony/dotenv|dotenv' || true
echo
echo "Search loadEnv calls with overrideExistingVars"
rg -n "new Dotenv|loadEnv\\(" src -S || trueRepository: phpList/core
Length of output: 377
🌐 Web query:
Symfony Dotenv 6.4 src loadEnv $defaultEnv $overrideExistingVars code
💡 Result:
Symfony Dotenv’s Dotenv::loadEnv($path, $envKey = null, $defaultEnv = 'dev', $testEnvs = ['test'], bool $overrideExistingVars = false) signature includes the $overrideExistingVars parameter as the last argument (defaulting to false). [1] The effect of $overrideExistingVars is that when the component populates variables, it will not skip already-set variables only if overrideExistingVars is true; otherwise, already-present $_ENV values are left untouched. [2] Symfony’s documented behavior for using overrideExistingVars is: - Passing overrideExistingVars: true to loadEnv will override environment variables defined by the system, but it will not override environment variables defined inside the.env files. [3] loadEnv also decides which.env files to load based on the app env key ($envKey or the component’s configured envKey) and whether variables are set/resolve to 'local'. It loads in this general order: 1) $path (or $path.dist if $path doesn’t exist) [4] 2) If the app env key is not defined yet, it populates it with $defaultEnv (default 'dev') and honors $overrideExistingVars when populating that env var. [4] 3) If the env is not in the $testEnvs list and $path.local exists, it loads $path.local. [4] 4) If the resolved env is exactly 'local', it returns early (so it won’t load the env-specific files). [4] 5) Otherwise it loads $path.$env and then $path.$env.local when those files exist. [4] Example usage (named argument): use Symfony\Component\Dotenv\Dotenv; $dotenv = new Dotenv; $dotenv->loadEnv(DIR.'/.env', overrideExistingVars: true); This is the documented call pattern for overriding system env vars. [3]
Citations:
- 1: https://github.com/symfony/dotenv/blob/6.2/Dotenv.php
- 2: https://github.com/symfony/dotenv/blob/7.0/Dotenv.php
- 3: https://symfony.com/doc/current/configuration.html
- 4: https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/Dotenv/Dotenv.php
🌐 Web query:
Symfony 6.4 Dotenv loadEnv does not override already set variable signature code
💡 Result:
In Symfony 6.4, the Dotenv component provides the loadEnv() method with an optional boolean parameter, $overrideExistingVars, which controls whether environment variables already set by the system should be overridden [1][2][3]. The method signature is: public function loadEnv(string $path,?string $envKey = null, string $defaultEnv = 'dev', array $testEnvs = ['test'], bool $overrideExistingVars = false): void By default, $overrideExistingVars is set to false [3][4]. When this parameter is false, loadEnv() will not override "real" environment variables (those already present in the system environment, such as those set by your web server or shell) [5][2]. However, it is important to note a specific nuance in how Symfony Dotenv handles variables: 1. Real Environment Variables vs. Dotenv Variables: "Real" environment variables (system-level) take precedence and are protected from being overwritten by .env files unless $overrideExistingVars is explicitly set to true [6][5][2]. 2. Precedence Within Dotenv Files: Variables defined within the series of .env files (e.g., .env, .env.local, .env.prod) follow a specific loading order where later-loaded files override values set in previously-loaded files [6][7]. This behavior applies to the Dotenv-managed variables themselves; the component is designed to ensure that more specific configuration files (like .env.local) take precedence over base configuration files [5]. If you need to force loadEnv() to override system-level environment variables, you must explicitly pass true to the overrideExistingVars argument: $dotenv = new Dotenv; $dotenv->loadEnv(DIR.'/.env', overrideExistingVars: true); As stated in the official documentation, this configuration allows you to override variables defined by the system, but it will not change the fundamental behavior where .env files maintain their own internal overriding logic [2].
Citations:
- 1: https://github.com/symfony/dotenv/blob/v6.4.8/CHANGELOG.md
- 2: https://symfony.com/doc/current/configuration.html
- 3: https://github.com/symfony/symfony/blob/8.0/src/Symfony/Component/Dotenv/Dotenv.php
- 4: https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/Dotenv/Dotenv.php
- 5: https://symfony.com/blog/improvements-to-the-handling-of-env-files-for-all-symfony-versions
- 6: [DotEnv] loading priority and overriding symfony/symfony#31579
- 7: Problem of overload and load in Dotenv have no different result symfony/symfony#62660
🌐 Web query:
site:github.com/symfony/symfony v6.4 Dotenv.php loadEnv overrideExistingVars true defaultEnv
💡 Result:
In the context of the Symfony Dotenv component (v6.4 and others), the overrideExistingVars parameter in the loadEnv() method controls whether the component overrides environment variables that were already set by the system (e.g., in the web server or OS) before the Dotenv files were loaded [1][2][3]. It is important to understand that this flag does not control the behavior between multiple .env files [4]. Within the Symfony Dotenv component, variables defined in loaded .env files will always override variables defined in previously loaded .env files, regardless of the value set for overrideExistingVars [5][4]. The overrideExistingVars flag specifically dictates whether these values can overwrite "real" environment variables that were already present in the environment before loadEnv() was called [4]. Key points regarding loadEnv and overrideExistingVars: - Purpose: The flag determines if system-level environment variables (e.g., those set by putenv or the server environment) can be replaced by values found in .env files [4]. - Scope: It does not prevent one .env file from overriding another. The loading sequence (e.g., .env -> .env.local -> .env.app_env) defines the priority for file-based variables, where later files take precedence over earlier ones [1][2]. - Default: By default, overrideExistingVars is set to false, meaning system-level environment variables will take precedence over those defined in your .env files [1][3]. - Technical behavior: When set to false, if an environment variable already exists in $_ENV or $_SERVER, the Dotenv loader will skip overwriting it [6][5]. If you require the values in your .env files to override system-level variables, you must explicitly set overrideExistingVars to true [4][7].
Citations:
- 1: https://github.com/symfony/symfony/blob/8.1/src/Symfony/Component/Dotenv/Dotenv.php
- 2: https://github.com/symfony/symfony/blob/8.0/src/Symfony/Component/Dotenv/Dotenv.php
- 3: https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/Dotenv/Dotenv.php
- 4: [Dotenv] Update README with variable override note symfony/symfony#52090
- 5: Problem of overload and load in Dotenv have no different result symfony/symfony#62660
- 6: [Dotenv] Env variable is override with $overrideExistingVars set on false symfony/symfony#57910
- 7: [Runtime] Add dotenv_overload option to SymfonyRuntime to tell Dotenv to override existing vars symfony/symfony#41681
Keep the kernel environment aligned with APP_ENV.
Dotenv::loadEnv() uses APP_ENV to load environment-specific files, but configureApplicationKernel() still passes the current $this->environment. If .env sets a different APP_ENV, .env.* is loaded for that environment while the kernel runs as the old environment, which produces a mismatched config/cache/runtime state. Read and validate the resolved APP_ENV after loading dotenv, then assign it to $this->environment before kernel creation.
Proposed fix
(new Dotenv())->loadEnv($dotenvPath, 'APP_ENV', $this->environment);
+$environment = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $this->environment;
+Environment::validateEnvironment($environment);
+$this->environment = $environment;
+
$secret = $_SERVER['PHPLIST_SECRET'] ?? $_ENV['PHPLIST_SECRET'] ?? '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Core/Bootstrap.php` at line 181, Update configureApplicationKernel around
Dotenv::loadEnv() to read the resolved APP_ENV after dotenv loading, validate
that it is a supported environment, and assign it to $this->environment before
creating the kernel. Ensure the kernel receives this resolved value so
environment-specific files and runtime configuration remain aligned.
2 participants
Summary by CodeRabbit
New Features
.env-based configuration for database, mail, security, messaging, uploads, and other application settings..envfile with a secure secret.Documentation
Thanks for contributing to phpList!