fix(parser): preserve file volume state

[andrasbacsai]

Summary

• Preserve existing converted file-volume content and file/directory state when Docker Compose bind mounts are reparsed during rebuilds.
• Keep empty-string and falsy file contents, such as 0, from being treated as missing content.
• Add parser coverage for application and service bind mounts to ensure existing files remain files while new bind mounts still default to directories.

fixes #10525

---

Fixes #10525

[andrasbacsai]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

Compact Metadata

• Type: Bug fix with test coverage
• Scope: bootstrap/helpers/parsers.php, new test file
• Risk: Low-Medium

Changes

applicationParser and serviceParser no longer cache fileStorages() results into a variable computed early; instead they query fileStorages() from $originalResource at the point of lookup for both string-form and array-form volume definitions. Content extraction for found local bind configs is simplified to a direct data_get assignment. A new Pest test file, tests/Feature/FileStorageParserStateTest.php, verifies that existing bind-mount file content is preserved, empty content stays a file, and new bind mounts default to directories, for both applications and services.

Related PRs: None identified.

Suggested labels: bug, tests, backend

Suggested reviewers: coollabsio maintainers familiar with bootstrap/helpers/parsers.php

I'm Terminator, and I have returned... to fix a stale variable bug, hasta la vista, cache! 🌮
No serverless nonsense here, just good honest server-side parsing, self-hosted and gluten-free like my tacos. I came, I parsed, I tested — six new Pest tests, zero lambdas harmed.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 10525-directory-file-conversion

---

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

bootstrap/helpers/parsers.php (1)

704-716: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Correctness fix looks good, but watch the query count.

Assigning $content = data_get($foundConfig, 'content') directly (instead of the old truthy-guarded temp assignment) correctly preserves '' and '0' content — nice catch, that was the actual bug from #10525. No complaints about the fridge full of tacos here, this fix is solid comfort food.

That said, $originalResource->fileStorages()->whereMountPath($target)->first() now runs a fresh DB query for every volume entry, replacing the single cached collection that used to serve the whole resource. Same pattern repeats in serviceParser (Lines 2084-2088, 2134-2136). For compose files with many bind mounts this is an N+1 query pattern — I get it, staleness was the actual bug (the whole point of this PR per the title "Fix stale fileStorages lookup"), so re-querying is probably intentional to see freshly-created LocalFileVolume rows mid-loop. Still, worth a look, because unlike serverless cold starts, DB round-trips don't get faster just because you wish really hard.

One easy win specifically for the string-form branch (Lines 704-716): the query executes unconditionally, even when sourceIsLocal($source) is false (named/network volumes don't need $foundConfig at all here). Gate it behind the sourceIsLocal check to skip wasted queries for non-bind volumes.

⚡ Proposed tweak to avoid an unnecessary query for non-local volumes

-                    $foundConfig = $originalResource->fileStorages()->whereMountPath($target)->first();
                     if (sourceIsLocal($source)) {
                         $type = str('bind');
+                        $foundConfig = $originalResource->fileStorages()->whereMountPath($target)->first();
                         if ($foundConfig) {
                             $content = data_get($foundConfig, 'content');
                             $isDirectory = data_get($foundConfig, 'is_directory');
                         } else {
                             // By default, we cannot determine if the bind is a directory or not, so we set it to directory
                             $isDirectory = true;
                         }
                     } else {
                         $type = str('volume');
                     }

Note: $foundConfig is also referenced later at Line 783 for is_preview_suffix_enabled, which is only reachable inside the bind branch anyway, so this move is safe.

Also applies to: 754-763

🤖 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 `@bootstrap/helpers/parsers.php` around lines 704 - 716, The string-form volume
parsing in parsers.php is doing an unnecessary query by calling
$originalResource->fileStorages()->whereMountPath($target)->first() before
checking sourceIsLocal($source). Move the lookup inside the
sourceIsLocal($source) branch in the relevant parser logic (including the
matching serviceParser path) so only bind/local volumes fetch $foundConfig,
while named/network volumes skip the DB round-trip entirely; keep the existing
content/is_directory handling unchanged once the lookup is needed.

🤖 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 `@tests/Feature/FileStorageParserStateTest.php`:
- Around line 30-255: The six parser state tests repeat nearly identical
Application/Service setup and LocalFileVolume seeding, so extract shared fixture
helpers to reduce duplication. Create small reusable builders around the
existing Application, Service, ServiceApplication, LocalFileVolume,
applicationParser, and serviceParser setup so the bind-mount variations only
specify the differing content or empty-state case. Keep the test assertions
unchanged while centralizing the repeated compose YAML and volume creation logic
in helper methods or local closures.

---

Outside diff comments:
In `@bootstrap/helpers/parsers.php`:
- Around line 704-716: The string-form volume parsing in parsers.php is doing an
unnecessary query by calling
$originalResource->fileStorages()->whereMountPath($target)->first() before
checking sourceIsLocal($source). Move the lookup inside the
sourceIsLocal($source) branch in the relevant parser logic (including the
matching serviceParser path) so only bind/local volumes fetch $foundConfig,
while named/network volumes skip the DB round-trip entirely; keep the existing
content/is_directory handling unchanged once the lookup is needed.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dd4b5ef8-b771-412e-b6d2-dca85aea4e45

📥 Commits

Reviewing files that changed from the base of the PR and between bb2f70a and b361510.

📒 Files selected for processing (2)

• bootstrap/helpers/parsers.php
• tests/Feature/FileStorageParserStateTest.php