docs: add design-rationale commentary to core support classes
Enriches class-level docblocks and inline comments across the shared
utility classes with the "why" behind design decisions that aren't
obvious from reading the code alone:
- Cache two-tier rationale, atomic-write semantics, failure modes
- Curl single-use-per-instance rationale, default option choices
- Log wrapper rationale, redaction expectations for callers
- Database auto-migration philosophy, schema-versioning approach
- ServerResource flat-array rationale, interfaces[0]-only limit called
out for future maintainers, unit-conversion map
- ConfigureService why a sibling of ModuleFunctions, catalogue caching
policy, cp-in-constructor reasoning
Pure documentation — no code changes, all files remain lint-clean and
Pint-formatted.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Prophet731
@@ -3,11 +3,47 @@
|
||||
namespace WHMCS\Module\Server\VirtFusionDirect;
|
||||
|
||||
/**
|
||||
* Two-tier cache: uses Redis when the ext-redis extension is available, with an atomic
|
||||
* filesystem fallback stored in the system temp directory.
|
||||
* Two-tier cache: Redis when ext-redis is available, atomic filesystem fallback otherwise.
|
||||
*
|
||||
* WHY TWO TIERS
|
||||
* -------------
|
||||
* The module is deployed to every kind of WHMCS install — shared hosting, dedicated
|
||||
* VPS, bare-metal. Requiring Redis would exclude the long tail of smaller operators
|
||||
* who never installed the extension. But operators who DO have Redis get a huge
|
||||
* performance win for cross-request caching (PowerDNS zone lists, OS template
|
||||
* listings, traffic stats), so we opportunistically use it when present.
|
||||
*
|
||||
* The fallback is filesystem-based, using the OS temp directory. Writes are atomic
|
||||
* via the classic tmp-file + rename pattern so a process crash mid-write can never
|
||||
* corrupt an existing cache entry for another concurrent reader.
|
||||
*
|
||||
* EXPIRY SEMANTICS
|
||||
* ----------------
|
||||
* Redis: native SETEX — the key auto-expires on the server side.
|
||||
* Filesystem: we store a JSON envelope {expires, data} and check expiry on read,
|
||||
* deleting stale entries lazily. This means a cache with lots of expired entries
|
||||
* will slowly accumulate files until accessed — acceptable for the module's scale
|
||||
* (tens-to-hundreds of keys per install) but worth noting if someone ports this
|
||||
* to a higher-volume context.
|
||||
*
|
||||
* NAMESPACE
|
||||
* ---------
|
||||
* Every key is prefixed with "vfd:" to avoid collisions with anything else that
|
||||
* shares the Redis instance. Nested keys add their own sub-prefix (e.g.
|
||||
* "pdns:zones:<hash>" for PowerDNS zone lists) for semantic clarity in the logs.
|
||||
*
|
||||
* FAILURE MODES
|
||||
* -------------
|
||||
* Redis unreachable: we set $redisAvailable = false on first failure, which
|
||||
* permanently disables Redis for the rest of this PHP process (subsequent calls
|
||||
* skip straight to the file cache). Prevents paying reconnect overhead on every
|
||||
* miss when Redis is down.
|
||||
* File cache write fails: silently skipped. Cache is best-effort; a failed SET
|
||||
* just means the next GET will re-fetch from the authoritative source.
|
||||
*/
|
||||
class Cache
|
||||
{
|
||||
/** Module-global key prefix — keeps us out of Redis key collisions on shared installs. */
|
||||
const PREFIX = 'vfd:';
|
||||
|
||||
/** @var \Redis|null */
|
||||
@@ -150,12 +186,18 @@ class Cache
|
||||
}
|
||||
}
|
||||
|
||||
// File cache fallback with atomic write (race condition safe)
|
||||
// File cache fallback with atomic write.
|
||||
// Writing to a temp file + rename ensures that readers either see the
|
||||
// complete previous entry or the complete new entry — never a truncated
|
||||
// or partially-written file. getmypid() suffix lets concurrent PHP
|
||||
// processes write to the same key without stomping each other's temp files.
|
||||
$path = self::filePath($key);
|
||||
$tmp = $path . '.' . getmypid() . '.tmp';
|
||||
$entry = json_encode(['expires' => time() + $ttl, 'data' => $value]);
|
||||
|
||||
if (@file_put_contents($tmp, $entry, LOCK_EX) !== false) {
|
||||
// rename() is atomic on POSIX when source and target are on the same
|
||||
// filesystem (which they always are here — both in sys_get_temp_dir).
|
||||
@rename($tmp, $path);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,9 +8,37 @@ use WHMCS\User\User;
|
||||
/**
|
||||
* Handles order-time and provisioning-time operations for VirtFusion servers.
|
||||
*
|
||||
* Extends Module to provide package discovery, OS template fetching, server build
|
||||
* initialization, and SSH key retrieval/creation. Used during WHMCS checkout and
|
||||
* account creation flows rather than ongoing service management.
|
||||
* WHY A SIBLING OF ModuleFunctions RATHER THAN METHODS ON IT
|
||||
* ----------------------------------------------------------
|
||||
* ModuleFunctions handles the WHMCS LIFECYCLE (create, suspend, terminate, etc.)
|
||||
* — operations driven by WHMCS service-state transitions.
|
||||
*
|
||||
* ConfigureService handles ORDER-TIME logic — package lookups, template fetching,
|
||||
* SSH key creation, initial build triggering. These run during checkout (via the
|
||||
* ClientAreaFooterOutput hook that populates dropdowns on the order form) and
|
||||
* immediately after account creation (initServerBuild is called from
|
||||
* ModuleFunctions::createAccount once the VirtFusion server exists).
|
||||
*
|
||||
* Splitting the concerns keeps ModuleFunctions focused on lifecycle state machines
|
||||
* and ConfigureService focused on catalogue/discovery calls. They share the base
|
||||
* Module's API plumbing via inheritance.
|
||||
*
|
||||
* CACHING
|
||||
* -------
|
||||
* Package/template lookups use the module's Cache class with 10-minute TTLs.
|
||||
* These values change rarely (a package list is typically edited once per
|
||||
* month at most) but the endpoints are on the checkout hot path, so aggressive
|
||||
* caching matters for page-load performance.
|
||||
*
|
||||
* CP RESOLVED IN CONSTRUCTOR
|
||||
* --------------------------
|
||||
* Unlike ModuleFunctions which resolves the control panel per-request via the
|
||||
* service ID, ConfigureService resolves it ONCE in the constructor via
|
||||
* getCP(false, true) — "any available VirtFusion server". Order-time operations
|
||||
* happen BEFORE a WHMCS service exists, so we can't dereference a specific
|
||||
* server through mod_virtfusion_direct. "Any enabled server" is the pragmatic
|
||||
* default for catalogue operations that typically return the same data
|
||||
* regardless of which panel you hit.
|
||||
*/
|
||||
class ConfigureService extends Module
|
||||
{
|
||||
|
||||
@@ -17,7 +17,20 @@ class Curl
|
||||
/** @var array User-supplied cURL options that override defaults */
|
||||
private $customOptions = [];
|
||||
|
||||
/** @var array Default cURL options applied to every request */
|
||||
/**
|
||||
* @var array Default cURL options applied to every request.
|
||||
*
|
||||
* Rationale:
|
||||
* VERIFYPEER/VERIFYHOST: Full TLS chain + hostname validation. Disabling
|
||||
* either is a common source of MITM bugs, so we never do it silently.
|
||||
* RETURNTRANSFER: We always want the response body back as a string.
|
||||
* HEADER off: Callers almost never need headers. Saves a parse cycle.
|
||||
* NOBODY off: Default to GET-style body-returning requests.
|
||||
* TIMEOUT 30s: Covers slow API endpoints without letting a hung connection
|
||||
* block a whole WHMCS request indefinitely.
|
||||
* CONNECTTIMEOUT 10s: Separate from the total timeout so a failed TCP
|
||||
* handshake (firewall black-hole) fails fast rather than burning 30s.
|
||||
*/
|
||||
private $defaultOptions = [
|
||||
CURLOPT_SSL_VERIFYPEER => true,
|
||||
CURLOPT_SSL_VERIFYHOST => 2,
|
||||
|
||||
@@ -7,12 +7,45 @@ use WHMCS\Database\Capsule as DB;
|
||||
/**
|
||||
* Handles all database operations for the module's custom table (mod_virtfusion_direct)
|
||||
* and queries against core WHMCS tables (tblhosting, tblclients, tblservers, etc.).
|
||||
*
|
||||
* SCHEMA AUTO-MIGRATION
|
||||
* ---------------------
|
||||
* schema() runs on every Module construction — the first call per request creates
|
||||
* or migrates the module table and ensures all required custom fields exist on
|
||||
* every VirtFusionDirect product. Subsequent calls within the same request hit
|
||||
* the $fieldsChecked idempotency flag and short-circuit, so the overhead is
|
||||
* one SHOW-columns query per request.
|
||||
*
|
||||
* This design means operators never need to run a separate install script —
|
||||
* dropping the module files into place and hitting any admin page triggers the
|
||||
* migration. The trade-off is small per-request overhead; we take it because
|
||||
* WHMCS modules historically had fragile install/uninstall hooks.
|
||||
*
|
||||
* SCHEMA VERSIONING
|
||||
* -----------------
|
||||
* No explicit version table. Migrations are expressed as "create if missing"
|
||||
* checks — hasTable(), hasColumn() — which makes forward migration additive
|
||||
* and safe to re-run. Deletions would require a proper versioning scheme, but
|
||||
* we have none so far; every column added has been non-breaking.
|
||||
*
|
||||
* WHMCS TABLE ACCESS
|
||||
* ------------------
|
||||
* Reads from tblhosting / tblclients / tblconfiguration are done via Capsule's
|
||||
* fluent query builder, not raw SQL, to inherit WHMCS's database abstraction
|
||||
* (connection pooling, character set, prepared statement handling).
|
||||
*/
|
||||
class Database
|
||||
{
|
||||
/** Module's own per-service state table. Created on first Module instantiation. */
|
||||
const SYSTEM_TABLE = 'mod_virtfusion_direct';
|
||||
|
||||
/** @var bool Tracks whether custom field existence has already been verified this request. */
|
||||
/**
|
||||
* @var bool Tracks whether custom field existence has already been verified this request.
|
||||
*
|
||||
* Custom-field creation is idempotent (updateOrInsert) but touching every
|
||||
* product on every request is wasteful. This flag ensures it runs exactly
|
||||
* once per PHP request.
|
||||
*/
|
||||
private static $fieldsChecked = false;
|
||||
|
||||
/**
|
||||
|
||||
@@ -3,18 +3,46 @@
|
||||
namespace WHMCS\Module\Server\VirtFusionDirect;
|
||||
|
||||
/**
|
||||
* Thin wrapper around the WHMCS logModuleCall() function for module-level logging.
|
||||
* Thin wrapper around the WHMCS logModuleCall() function.
|
||||
*
|
||||
* WHY A WRAPPER
|
||||
* -------------
|
||||
* Consolidating log writes lets us:
|
||||
* - Pin the module name in one place (the LOG_MODULE constant). All entries
|
||||
* go under "VirtFusionDirect" regardless of which caller inserted them,
|
||||
* which keeps WHMCS Admin → Utilities → Logs → Module Log filterable.
|
||||
* - Get a stable import path for every file that logs (Log::insert).
|
||||
* - Add cross-cutting policy later (e.g. redaction, sampling) without
|
||||
* touching every call site.
|
||||
*
|
||||
* OUTPUT SURFACE
|
||||
* --------------
|
||||
* Entries appear in WHMCS Admin → Utilities → Logs → Module Log. The request
|
||||
* and response parameters accept strings OR arrays — WHMCS serialises arrays
|
||||
* to readable form automatically. Pass structured data (["zone" => $z, "ip" => $ip])
|
||||
* rather than string-concatenated messages; the UI renders arrays as key/value
|
||||
* pairs which makes filtering and debugging much easier.
|
||||
*
|
||||
* REDACTION EXPECTATION
|
||||
* ---------------------
|
||||
* Callers are responsible for not passing secrets into logs. In particular:
|
||||
* - Never log Authorization/X-API-Key headers
|
||||
* - Never log full request_header info from the Curl class
|
||||
* - Never log the decrypted VirtFusion bearer token or PowerDNS API key
|
||||
* The Curl class deliberately defaults CURLOPT_HEADER to off so header capture
|
||||
* doesn't accidentally populate a field that callers might log.
|
||||
*/
|
||||
class Log
|
||||
{
|
||||
/** Keep this in sync with the WHMCS server module name, so filters work. */
|
||||
const LOG_MODULE = 'VirtFusionDirect';
|
||||
|
||||
/**
|
||||
* Write an entry to the WHMCS module log.
|
||||
*
|
||||
* @param string $action Name of the action being logged (e.g. 'CreateAccount')
|
||||
* @param string|array $requestString Request data sent to the API
|
||||
* @param string|array $responseData Response data received from the API
|
||||
* @param string $action Short tag identifying the operation (used as the "Function" column in the log UI)
|
||||
* @param string|array $requestString Outbound payload or context data. Arrays preferred — rendered as key/value pairs.
|
||||
* @param string|array $responseData Inbound response or result. Same conventions as $requestString.
|
||||
*/
|
||||
public static function insert($action, $requestString, $responseData)
|
||||
{
|
||||
|
||||
@@ -4,6 +4,47 @@ namespace WHMCS\Module\Server\VirtFusionDirect;
|
||||
|
||||
/**
|
||||
* Transforms a VirtFusion API server response into a flat key-value array for Smarty templates and admin display.
|
||||
*
|
||||
* WHY A FLAT ARRAY
|
||||
* ----------------
|
||||
* Smarty templates can traverse nested structures (`{$data.network.interfaces[0].ipv4[0].address}`)
|
||||
* but that leaks the API shape into the template layer. A flat array ("hostname",
|
||||
* "primaryNetwork.ipv4[]", "memoryRaw", etc.) decouples the template from the upstream
|
||||
* schema: if VirtFusion renames `network.interfaces` tomorrow, only this file needs
|
||||
* to change.
|
||||
*
|
||||
* PRIMARY-INTERFACE-ONLY DESIGN
|
||||
* -----------------------------
|
||||
* process() only reads interfaces[0]. That's the primary network — the one the
|
||||
* client-area "Overview" card displays. Servers with multiple interfaces (common
|
||||
* for dedicated IPMI networks, storage networks, etc.) still work for display
|
||||
* because the primary interface holds the customer-facing IP.
|
||||
*
|
||||
* The reverse-DNS subsystem (PowerDns\IpUtil::extractIps) walks ALL interfaces
|
||||
* explicitly because PTRs matter for every IP no matter which NIC it's on.
|
||||
* If you add a feature that needs secondary-interface data for display, do NOT
|
||||
* generalise this class — add a new one or a helper that doesn't disturb the
|
||||
* well-tested primary-interface behaviour.
|
||||
*
|
||||
* UNIT CONVERSIONS
|
||||
* ----------------
|
||||
* VirtFusion stores:
|
||||
* - traffic as bytes (usage) or GB (limits)
|
||||
* - storage as GB (limits) or bytes (usage)
|
||||
* - memory as MB
|
||||
* WHMCS expects MB for storage/traffic in tblhosting. This class produces two
|
||||
* pairs of values per resource: a human-readable string with unit suffix
|
||||
* (e.g. "200 GB") AND a raw integer without the unit (for slider UIs and
|
||||
* arithmetic). Keep both — removing one breaks a UI consumer somewhere.
|
||||
*
|
||||
* "-" SENTINELS
|
||||
* -------------
|
||||
* Fields that are missing or empty are rendered as "-" rather than empty strings.
|
||||
* That makes the client-area card always have content (a dash is a valid visual
|
||||
* placeholder) and distinguishes "missing data" from "empty string returned by
|
||||
* the API". Consumers who need boolean presence checks should test against "-",
|
||||
* not "" / null — and upstream (e.g. updateWhmcsServiceParamsOnServerObject)
|
||||
* already does.
|
||||
*/
|
||||
class ServerResource
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user