fix: use port range on free allocation lookup

[PalmarHealer]

Updated free allocation search logic to respect port range.

Supersedes #1852

[coderabbitai]

📝 Walkthrough

Walkthrough

Service reads allocation range_start/range_end from config, asserts they are integerish, applies whereBetween('port', [$start, $end]) when querying allocations, and passes ?int $start, ?int $end into createNewAllocation. createNewAllocation no longer reads or asserts config; exception behavior for missing/exhausted ranges remains.

Changes

Cohort / File(s)	Summary
Port Range & Allocation Flow app/Services/Allocations/FindAssignableAllocationService.php	Reads allocation.range_start/allocation.range_end from config and asserts with Assert::integerish; applies whereBetween('port', [$start, $end]) to allocation queries; updates createNewAllocation signature to createNewAllocation(Server $server, ?int $start, ?int $end) and passes the range when creating allocations; removes internal config retrieval and assertions from createNewAllocation; preserves NoAutoAllocationSpaceAvailableException behavior.
Integration Test Adjustment tests/Integration/Services/Allocations/FindAssignableAllocationServiceTest.php	Configures allocation range to 5000-5005 in the test and assigns port 5005 to a pre-created allocation to exercise the existing-allocation path within the new range.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as FindAssignableAllocationService
  participant DB as Allocations DB
  participant Creator as createNewAllocation

  Caller->>Caller: read config allocation.range_start / range_end\nAssert::integerish(start,end)
  Caller->>DB: query allocations where server_id = X\nand whereBetween(port, [start,end])
  alt allocation found
    DB-->>Caller: return Allocation
  else none found
    Caller->>Creator: createNewAllocation(server, start, end)
    Creator-->>DB: insert allocation with port in [start,end]
    DB-->>Creator: return new Allocation
    Creator-->>Caller: return Allocation
  end

Loading

Possibly related PRs

• fix: bypass tenant scoping in allocation queries #1883 — Modifies allocation lookup logic in the same service file; adds related query changes (adds withoutGlobalScopes() to allocation queries).

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title directly summarizes the main change: using port range on free allocation lookup, which aligns with the core modification across the codebase.
Description check	✅ Passed	The pull request description is directly related to the changeset, describing the update to free allocation search logic to respect port range.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

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 and usage tips.}

[PalmarHealer]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

✅ Actions performed

Review triggered.

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]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ee27e5 and da38612.

📒 Files selected for processing (1)

• app/Services/Allocations/FindAssignableAllocationService.php (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-12T17:33:57.388Z

Learnt from: lancepioch
Repo: pelican-dev/panel PR: 1595
File: app/Services/Servers/ServerCreationService.php:182-184
Timestamp: 2025-08-12T17:33:57.388Z
Learning: In the Pelican Panel codebase, when updating allocations to assign them to servers, the preferred pattern is to use whereNull('server_id'), lockForUpdate(), and direct property assignment with save() rather than mass update methods, to prevent race conditions and mass-assignment issues while ensuring model events fire properly.

Applied to files:

• app/Services/Allocations/FindAssignableAllocationService.php

🧬 Code graph analysis (1)

app/Services/Allocations/FindAssignableAllocationService.php (2)

app/Models/Server.php (4)

• allocation (277-280)
• node (343-346)
• allocations (287-290)
• Server (132-516)

app/Models/Allocation.php (3)

• server (129-132)
• node (137-140)
• Allocation (48-141)

🔇 Additional comments (2)

app/Services/Allocations/FindAssignableAllocationService.php (2)

59-59: LGTM: Proper null coalescing assignment.

The null coalescing assignment operator correctly falls back to creating a new allocation only when no existing allocation is found.

---

77-81: Good refactoring: Parameters moved to method signature.

The refactoring to accept $start and $end as parameters instead of reading config internally is cleaner and makes the method more testable. The null check at line 79 properly validates that the range is configured before attempting to create a new allocation.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/Integration/Services/Allocations/FindAssignableAllocationServiceTest.php (1)

32-38: LGTM! Changes correctly align test with range-based allocation logic.

The updates ensure the test works with the new port range filtering:

• Configures a valid range (5000-5005)
• Creates an allocation with port 5005 (within range)
• Tests inclusive upper bound behavior

Optional refinement for consistency:

Consider adding an explicit port assertion for consistency with other tests (lines 64, 86):

 $this->assertSame($created->id, $response->id);
+$this->assertSame(5005, $response->port);
 $this->assertSame($server->allocation->ip, $response->ip);

While the port is implicitly verified by the id match, an explicit assertion would make the test more self-documenting and consistent with the pattern used in test_new_allocation_is_created_if_one_is_not_found and test_only_port_not_in_use_is_created.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfaa147 and 9c64592.

📒 Files selected for processing (1)

• tests/Integration/Services/Allocations/FindAssignableAllocationServiceTest.php (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-12T17:33:57.388Z

Learnt from: lancepioch
Repo: pelican-dev/panel PR: 1595
File: app/Services/Servers/ServerCreationService.php:182-184
Timestamp: 2025-08-12T17:33:57.388Z
Learning: In the Pelican Panel codebase, when updating allocations to assign them to servers, the preferred pattern is to use whereNull('server_id'), lockForUpdate(), and direct property assignment with save() rather than mass update methods, to prevent race conditions and mass-assignment issues while ensuring model events fire properly.

Applied to files:

• tests/Integration/Services/Allocations/FindAssignableAllocationServiceTest.php

🧬 Code graph analysis (1)

tests/Integration/Services/Allocations/FindAssignableAllocationServiceTest.php (2)

app/Models/Allocation.php (1)

• Allocation (48-141)

app/Models/Server.php (1)

• allocation (277-280)