Do not deploy ExApp environment variables with an empty default as the literal string Array

[oleksandr-nc]

An empty <default></default> (or <default/>) element for a declared environment variable is parsed into an empty array by the simplexml/json roundtrip, slips past the empty-value filter and ends up in the container as the literal string Array, for both Docker and Kubernetes deployments.

Declared variables now go through a small ExAppEnvVarsHelper (same pattern as ExAppRouteHelper): every field is normalized to a string, so empty elements become empty strings and the existing filter drops them. A missing or empty <name> is rejected with a readable registration error instead of a fatal TypeError. Values already persisted in deploy options from earlier registrations heal on the next update, since stored options re-enter this path as overrides.

Covered by unit tests at both the helper and the getAppInfo level, including cases that round-trip the real XML shapes from the issue.

Fixes #969

[coderabbitai]

📝 Walkthrough

Walkthrough

Added ExAppEnvVarsHelper to normalize and validate ExApp environment-variable definitions. The helper converts scalar values to strings, applies declared-name overrides, removes empty final values, and rejects invalid entries. ExAppService::getAppInfo() now uses the helper for XML and JSON input and returns descriptive errors for invalid input. PHPUnit tests cover empty defaults, overrides, numeric values, undeclared variables, malformed definitions, and empty variable names.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: preventing empty ExApp environment-variable defaults from becoming the literal string Array.
Description check	✅ Passed	The description accurately explains the parsing defect, normalization fix, affected deployment paths, error handling, and test coverage.
Linked Issues check	✅ Passed	The changes address issue #969 by normalizing empty defaults and overrides, omitting empty values, and covering XML and JSON registration paths.
Out of Scope Changes check	✅ Passed	The added validation, override normalization, JSON support, and tests directly support the linked issue and stated pull request objectives.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f3f1a88d-6245-4a39-889c-85d8cc1f6a92

📥 Commits

Reviewing files that changed from the base of the PR and between 65d78ff and e009b0d.

📒 Files selected for processing (4)

• lib/Service/ExAppEnvVarsHelper.php
• lib/Service/ExAppService.php
• tests/php/Service/ExAppEnvVarsHelperTest.php
• tests/php/Service/ExAppServiceGetAppInfoTest.php

[coderabbitai]

🧹 Nitpick comments (2)

tests/php/Service/ExAppServiceGetAppInfoTest.php (2)

159-185: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Exercise empty JSON values that become empty arrays.

The test uses default => '', but the regression input can arrive as [] after XML/JSON round-tripping. Add a JSON variable with default => []. Also add a declared variable with an empty deploy-option override and assert that both variables are absent from the result.

Suggested test additions

 						['name' => 'EMPTY', 'display-name' => 'Empty', 'default' => ''],
 						['name' => 'KEPT', 'display-name' => 'Kept', 'default' => 'v'],
+						['name' => 'EMPTY_OVERRIDE', 'default' => 'v'],
 						['name' => 'TYPED', 'default' => 5],
@@
-			'environment_variables' => ['KEPT' => 'overridden'],
+			'environment_variables' => [
+				'KEPT' => 'overridden',
+				'EMPTY_OVERRIDE' => '',
+			],

Change EMPTY to use default => [], or add a separate array-valued case.

The PR objective requires empty defaults and empty overrides to be omitted for JSON-info registrations.

---

207-223: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the complete invalid-name error contract.

This test covers an empty name but not a missing name key. Add a separate missing-name case. Replace the generic assertion with a field-specific assertion such as:

Suggested assertion

-		self::assertStringContainsString('invalid environment variable definition', $appInfo['error']);
+		self::assertStringContainsString("'name' must be a non-empty string", $appInfo['error']);

The PR objective requires readable errors for both missing and empty names.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c59a46cc-b738-42c5-9f94-06fbf828363c

📥 Commits

Reviewing files that changed from the base of the PR and between e009b0d and e0c015a.

📒 Files selected for processing (2)

• lib/Service/ExAppService.php
• tests/php/Service/ExAppServiceGetAppInfoTest.php

🚧 Files skipped from review as they are similar to previous changes (1)

• lib/Service/ExAppService.php