refactor: remove/rename helpers, test --fn mode, param defaults, WGCTL_BINARY

commands/remove.command.sh

@@ -82,39 +82,20 @@ function cmd::remove::run() {
   local was_blocked=false
   peers::is_blocked "$name" && was_blocked=true
 
-  # Unapply rule if assigned
+  cmd::remove::_cleanup "$name" "$client_ip" "$was_blocked" || return 1
-  local assigned_rule
-  assigned_rule=$(peers::get_meta "$name" "rule")
-
-  if [[ -z "$assigned_rule" ]]; then
-    assigned_rule=$(peers::default_rule "$name")
-  fi
-
-  # Flush all iptables rules for this peer IP
-  if [[ -n "$client_ip" ]]; then
-    fw::flush_peer "$client_ip"
-  fi
-
-  # Remove peer from server config
-  peers::remove_from_server "$name" || return 1
-
-  # Remove client config
-  peers::remove_client_config "$name" || return 1
-
-  # Remove keys
-  keys::remove "$name" || return 1
-
-  # Remove block rules only if client was fully blocked
-  if [[ -n "$client_ip" ]] && $was_blocked; then
-    fw::unblock_all "$client_ip"
-  fi
-
-  fw::remove_block_file "$name" 2>/dev/null || true
-
-  peers::remove_meta "$name" 2>/dev/null || true
-
-  # Reload WireGuard
-  peers::reload || return 1
-
   log::wg_success "Client removed: ${name}"
 }
+
+function cmd::remove::_cleanup() {
+  local name="${1:-}" client_ip="${2:-}" was_blocked="${3:-false}"
+  
+  [[ -n "$client_ip" ]] && fw::flush_peer "$client_ip"
+  peers::remove_from_server "$name"     || return 1
+  peers::remove_client_config "$name"   || return 1
+  keys::remove "$name"                  || return 1
+
+  [[ -n "$client_ip" ]] && $was_blocked && fw::unblock_all "$client_ip"
+  fw::remove_block_file "$name"         2>/dev/null || true
+  peers::remove_meta "$name"            2>/dev/null || true
+  peers::reload                         || return 1
+}

commands/rename.command.sh

@@ -70,15 +70,7 @@ function cmd::rename::run() {
   fi
 
   name=$(peers::resolve_and_require "$name" "$type") || return 1
-  
-  # Resolve new name if provided
-  if [[ -n "$new_name" || -n "$new_type" ]]; then
-    # If only new_type provided, keep same base name
-    if [[ -z "$new_name" ]]; then
-      new_name=$(echo "$name" | cut -d'-' -f2-)
-    fi
   new_name=$(peers::resolve_name "$new_name" "$new_type") || return 1
-  fi
 
   local dir
   dir="$(ctx::clients)"
@@ -95,33 +87,31 @@ function cmd::rename::run() {
 
   log::section "Renaming client: ${name} → ${new_name}"
 
-  # Rename client files
+  cmd::rename::_rename_files "$name" "$new_name"
-  mv "${dir}/${name}.conf"        "${dir}/${new_name}.conf"
-  mv "${dir}/${name}_private.key" "${dir}/${new_name}_private.key"
-  mv "${dir}/${name}_public.key"  "${dir}/${new_name}_public.key"
-
-  log::fs_write "Renamed client files"
-
-  # Update comment in wg0.conf
-  sed -i "s/^# ${name}$/# ${new_name}/" "$(config::config_file)"
-
-  log::fs_write "Updated server config"
-
-  # Update block file if exists
-  local block_file
-  block_file="$(ctx::block::path "${name}.block")"
-  if [[ -f "$block_file" ]]; then
-    mv "$block_file" "$(ctx::block::path "${new_name}.block")"
-    log::fs_write "Renamed block file"
-  fi
-
-  local old_meta new_meta
-  old_meta=$(peers::meta_path "$name")
-  new_meta=$(peers::meta_path "$new_name")
-  [[ -f "$old_meta" ]] && mv "$old_meta" "$new_meta"
 
   # Reload WireGuard
   peers::reload
 
   log::wg_success "Client renamed: ${name} → ${new_name}"
 }
+
+function cmd::rename::_rename_files() {
+  local name="${1:-}" new_name="${2:-}"
+  local dir
+  dir="$(ctx::clients)"
+
+  mv "${dir}/${name}.conf"        "${dir}/${new_name}.conf"
+  mv "${dir}/${name}_private.key" "${dir}/${new_name}_private.key"
+  mv "${dir}/${name}_public.key"  "${dir}/${new_name}_public.key"
+
+  sed -i "s/^# ${name}$/# ${new_name}/" "$(config::config_file)"
+
+  local block_file
+  block_file="$(ctx::block::path "${name}.block")"
+  [[ -f "$block_file" ]] && mv "$block_file" "$(ctx::block::path "${new_name}.block")"
+
+  local old_meta new_meta
+  old_meta=$(peers::meta_path "$name")
+  new_meta=$(peers::meta_path "$new_name")
+  [[ -f "$old_meta" ]] && mv "$old_meta" "$new_meta"
+}

commands/test.command.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-WGCTL_BINARY="/usr/local/bin/wgctl"
+WGCTL_BINARY="$(command -v wgctl)"
 
 # ============================================
 # Lifecycle
@@ -119,6 +119,8 @@ function cmd::test::run_function() {
     cmd::unblock::run)     cmd::test::fn_unblock      ;;
     cmd::remove::run)      cmd::test::fn_remove       ;;
     cmd::rule::assign)     cmd::test::fn_rule_assign  ;;
+    cmd::rename::run)      cmd::test::fn_rename       ;;
+    cmd::remove::run)      cmd::test::fn_remove       ;;
     *)
       log::error "No function test defined for: ${fn}"
       return 1
@@ -271,6 +273,55 @@ function cmd::test::fn_block() {
   "$WGCTL_BINARY" remove --name phone-testunit --force > /dev/null 2>&1 || true
 }
 
+function cmd::test::fn_remove() {
+  test::section "cmd::remove::run"
+
+  # Setup
+  "$WGCTL_BINARY" remove --name phone-testunit --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" add --name testunit --type phone \
+    > /dev/null 2>&1
+
+  # Tests
+  # Skip the interactive prompt test — it hangs waiting for input
+  # cmd::test::run_cmd_fails "remove without --force" \
+  #   remove --name phone-testunit
+  cmd::test::run_cmd "remove with --force" "removed" \
+    remove --name phone-testunit --force
+  cmd::test::run_cmd_fails "remove nonexistent" \
+    remove --name nonexistent-peer --force
+  cmd::test::run_cmd_fails "remove missing --name" \
+    remove --force
+
+  # Cleanup already done by tests
+}
+
+function cmd::test::fn_rename() {
+  test::section "cmd::rename::run"
+
+  # Setup
+  "$WGCTL_BINARY" remove --name phone-testunit --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" remove --name phone-testunit2 --force \
+    > /dev/null 2>&1 || true
+  "$WGCTL_BINARY" add --name testunit --type phone \
+    > /dev/null 2>&1
+
+  # Tests
+  cmd::test::run_cmd "rename peer" "renamed" \
+    rename --name phone-testunit --new-name phone-testunit2
+  cmd::test::run_cmd_fails "rename to existing" \
+    rename --name phone-testunit2 --new-name phone-nuno
+  cmd::test::run_cmd_fails "rename nonexistent" \
+    rename --name phone-nonexistent --new-name phone-testunit
+  cmd::test::run_cmd_fails "rename missing --new-name" \
+    rename --name phone-testunit2
+
+  # Cleanup
+  /usr/local/bin/wgctl remove --name phone-testunit2 --force \
+    > /dev/null 2>&1 || true
+}
+
 # ============================================
 # Run
 # ============================================