Skip to content

refactor: small updates for some services #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loadinglucian merged 1 commit into from
Oct 26, 2025
Merged

refactor: small updates for some services #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ vendor/
.env.*
*.cache
*.log
inventory.yml
deployer.yml
Thumbs.db
8 changes: 7 additions & 1 deletion app/Contracts/BaseCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use Bigpixelrocket\DeployerPHP\Container;
use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository;
use Bigpixelrocket\DeployerPHP\Repositories\SiteRepository;
use Bigpixelrocket\DeployerPHP\Services\DigitalOceanService;

@coderabbitai coderabbitai Bot Oct 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that these services are commonly needed across commands.

FilesystemService and DigitalOceanService are injected into BaseCommand but not used within it. While this is acceptable if most command subclasses need these services, injecting dependencies that are only used by a few commands can increase coupling and make testing more complex.

Run the following script to verify how many command subclasses actually use these services:

Also applies to: 12-12, 44-44, 54-56

🏁 Script executed:

#!/bin/bash
# Description: Check usage of FilesystemService and DigitalOceanService in command subclasses

echo "=== Commands extending BaseCommand ==="
ast-grep --pattern 'class $_ extends BaseCommand { $$$ }'

echo -e "\n=== Usage of \$this->fs in commands ==="
rg -n --type=php -C2 '\$this->fs' -g '*Command.php' -g '!BaseCommand.php'

echo -e "\n=== Usage of \$this->digitalOcean in commands ==="
rg -n --type=php -C2 '\$this->digitalOcean' -g '*Command.php' -g '!BaseCommand.php'

Length of output: 128

I need to investigate further since the previous script produced no output. Let me try different approaches to locate commands and verify the service usage.

🏁 Script executed:

#!/bin/bash
# Description: Find Command files and verify BaseCommand structure

echo "=== Find all *Command.php files ==="
fd -t f -e php 'Command\.php$' | head -20

echo -e "\n=== Check BaseCommand.php structure ==="
cat app/Contracts/BaseCommand.php | head -80

echo -e "\n=== Search for classes extending BaseCommand (broader pattern) ==="
rg -n --type=php 'extends\s+BaseCommand' | head -20

echo -e "\n=== Search for protected readonly properties ==="
rg -n --type=php 'protected\s+readonly' app/Contracts/BaseCommand.php

Length of output: 4341

🏁 Script executed:

#!/bin/bash
# Description: Check actual usage of fs and digitalOcean in command files

echo "=== Usage of fs property in commands ==="
rg -n --type=php 'fs' app/Console/*.php app/Console/**/*.php | grep -E '(->fs|this->fs|\$this->fs)' || echo "No matches found"

echo -e "\n=== Usage of digitalOcean property in commands ==="
rg -n --type=php 'digitalOcean' app/Console/*.php app/Console/**/*.php | grep -E '(->digitalOcean|this->digitalOcean|\$this->digitalOcean)' || echo "No matches found"

echo -e "\n=== Full content of HelloCommand.php (smallest command for inspection) ==="
cat app/Console/HelloCommand.php

echo -e "\n=== Full content of ServerAddCommand.php (likely uses services) ==="
cat app/Console/Server/ServerAddCommand.php

Length of output: 6295

Let me check the remaining command files to ensure comprehensive coverage:

🏁 Script executed:

#!/bin/bash
# Description: Verify all remaining commands for fs and digitalOcean usage

for file in app/Console/Server/ServerDeleteCommand.php app/Console/Server/ServerListCommand.php app/Console/Site/SiteAddCommand.php app/Console/Site/SiteDeleteCommand.php app/Console/Site/SiteListCommand.php; do
  echo "=== $file ==="
  grep -E '(\$this->fs|\$this->digitalOcean|->fs|->digitalOcean)' "$file" || echo "No usage found"
  echo ""
done

Length of output: 315

Remove unused FilesystemService and DigitalOceanService from BaseCommand constructor.

Comprehensive verification across all 7 command subclasses confirms neither FilesystemService ($this->fs) nor DigitalOceanService ($this->digitalOcean) are ever used. Injecting unused dependencies increases coupling, complicates testing, and violates clean code practices. Remove both services from the constructor and import statements.

🤖 Prompt for AI Agents 
In app/Contracts/BaseCommand.php around line 10, the FilesystemService and
DigitalOceanService imports and their injections in the BaseCommand constructor
are unused across subclasses; remove the use statements for both services,
delete the corresponding constructor parameters and any class properties ($fs
and $digitalOcean), and update the constructor signature and body to only accept
and assign the services actually used. After removal, run a project-wide search
for $this->fs and $this->digitalOcean to confirm there are no remaining
references and adjust unit tests or callers creating BaseCommand instances to
match the new constructor signature.

use Bigpixelrocket\DeployerPHP\Services\EnvService;
use Bigpixelrocket\DeployerPHP\Services\FilesystemService;
use Bigpixelrocket\DeployerPHP\Services\GitService;
use Bigpixelrocket\DeployerPHP\Services\InventoryService;
use Bigpixelrocket\DeployerPHP\Services\IOService;
Expand Down Expand Up @@ -39,6 +41,7 @@ public function __construct(

// Base services
protected readonly EnvService $env,
protected readonly FilesystemService $fs,
protected readonly GitService $git,
protected readonly InventoryService $inventory,
protected readonly IOService $io,
Expand All @@ -48,6 +51,9 @@ public function __construct(
protected readonly ServerRepository $servers,
protected readonly SiteRepository $sites,
protected readonly SSHService $ssh,

// Providers
protected readonly DigitalOceanService $digitalOcean,
) {
parent::__construct();
}
Expand All @@ -74,7 +80,7 @@ protected function configure(): void
'inventory',
null,
InputOption::VALUE_OPTIONAL,
'Custom path to inventory.yml file (defaults to inventory.yml in the current working directory)'
'Custom path to deployer.yml file (defaults to deployer.yml in the current working directory)'
);
}

Expand Down
2 changes: 1 addition & 1 deletion app/Services/EnvService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function get(array|string $keys, bool $required = true): ?string
if ($required) {
$list = implode(', ', $keysList);
$label = count($keysList) > 1 ? 'variables' : 'variable';
throw new \RuntimeException("Missing required environment {$label}: {$list}");
throw new \InvalidArgumentException("Missing required environment {$label}: {$list}");
}

return null;
Expand Down
5 changes: 5 additions & 0 deletions app/Services/IOService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,11 @@ public function promptSpin(
Closure $callback,
string $message = 'Loading...'
): mixed {
// Bypass spinner in test environment to prevent terminal conflicts in parallel execution
if (defined('PHPUNIT_COMPOSER_INSTALL') || defined('__PEST_RUNNING__')) {
return $callback();
}

return spin(
callback: $callback,
message: $message
Expand Down
2 changes: 1 addition & 1 deletion app/Services/InventoryService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ private function unsetByPath(array &$data, array $segments): bool
*/
private function getInventoryPath(): string
{
return $this->inventoryPath ?? rtrim($this->fs->getCwd(), '/') . '/inventory.yml';
return $this->inventoryPath ?? rtrim($this->fs->getCwd(), '/') . '/deployer.yml';
}

/**
Expand Down
Loading