refactor(sdk): OOP modernization — AbstractClient, HttpTransport, SessionCapable trait#167
Open
KaiSchwarz-cnic wants to merge 12 commits into
Open
refactor(sdk): OOP modernization — AbstractClient, HttpTransport, SessionCapable trait#167KaiSchwarz-cnic wants to merge 12 commits into
KaiSchwarz-cnic wants to merge 12 commits into
Conversation
Eliminates the ~30-line duplicate cURL block between CNR\Client and IBS\Client. The new protected executeCurl() handles handle init, option setup, execution, and error detection, returning [raw, error|null]. Each request() method retains only its brand-specific logic — command preparation, URL assembly, extra cURL options — and creates its own Response subtype directly, keeping the type hierarchy clean without factory indirection. Also fixes the latent bug in IBS\Client where debug logging passed secured=false, which would have exposed passwords in debug output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Uses PHP 8.0 match expression with comma-separated arms and throw expressions. Consolidates three separate setCustomLogger() calls into one ternary based on instanceof IBSSessionClient, which naturally covers both IBS and MONIKER (the latter extends the former). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and Record Applies PHP 8.1 constructor promotion to eliminate separate property declarations and constructor assignments. All properties are set once and never mutated, so readonly is added throughout. Column: $key and $data promoted to private readonly; $length remains a regular public readonly property since it is computed (count($data)) rather than assigned directly from a parameter. Record: $data promoted to private readonly; constructor body is now empty. The duplicate @var docblock is removed — the type is already declared on the promoted parameter. IBS\Column and IBS\Record are empty subclasses and require no changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…entFactory Adds CNIC\Registrar (backed string enum) covering all recognised registrar identifiers including the two EOL cases. ClientFactory now resolves the input via Registrar::tryFrom() and matches on enum cases, replacing the raw string literals. Unrecognised input resolves to null, which the match arm converts to the existing "not supported" exception. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both properties were public, allowing external code to read and mutate
internal configuration state directly. They are now protected with
public getSettings(): array and isOTE(): bool accessors.
Three test locations that mutated \$settings are updated:
- testGetUrl: reads via getSettings() instead of direct access
- testSetRemoteIpAddressSetThrows / Set: use a fresh SessionClient()
(no config = no ipfilter key) instead of unsetting from shared state,
removing a hidden ordering dependency between tests
- testRequestCurlExecFail2: chains ->useOTESystem()->setURL("bad-url")
rather than writing to settings; testRequestFlattenCommand simply
calls useOTESystem() to restore the correct URL from settings
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t LSP violation IBS\SessionClient had four methods (login, logout, saveSession, reuseSession) that unconditionally threw "Method not supported" — the class was named SessionClient but incapable of session communication. Fix: extract the real session implementations into a CNIC\CNR\SessionCapable trait. CNR\SessionClient uses the trait; IBS\SessionClient does not, so it no longer carries misleading stubs. MONIKER\SessionClient (which extends IBS) is unaffected — it also does not support sessions. The trait is the extension point for any future registrar that does support sessions: include it and the capability is available. Test adjusted: testSaveReuseSession instantiates SessionClient directly rather than via ClientFactory so PHPStan can infer the precise type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, #14) AbstractClient (CNIC namespace) is the shared foundation for all registrar API clients. It holds every property and method previously in CNR\Client that is not CNR-specific: configuration loading, transport setup, debug mode, user agent, proxy/referer, credentials, IDN conversion, OTE/LIVE switching, settings/isOTE accessors, and the executeCurl() bridge. Three abstract methods define the contract each concrete client must satisfy: request(), newSocketConfig(), and setDefaultLogger(). HttpTransport (CNIC namespace) owns the cURL handle lifecycle and exposes a single post() method. The $chandle property is removed from the client layer entirely; close() now delegates to the transport. CNR\Client extends AbstractClient and retains only what is CNR-specific: request() with CNR command formatting, requestNextResponsePage(), and requestAllResponsePages(). IBS\Client now extends AbstractClient directly instead of CNR\Client, correctly reflecting that IBS shares the common foundation but not the CNR-specific layer. The redundant constructor override is eliminated since AbstractClient already calls newSocketConfig() during construction. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tpTransport final; drop curl_close - \$logger is always set via abstract setDefaultLogger() called in AbstractClient::__construct() — suppression is justified since Psalm cannot trace through abstract method dispatch - HttpTransport is never extended and not public API → final - curl_close() is deprecated since PHP 8.5; CurlHandle is freed automatically by the GC when set to null Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SessionCapable::saveSession() and reuseSession(): add ': static'
return type declarations to match the fluent contract of every other
setter in AbstractClient (PHPStan was correct to flag the gap)
- IBS\Client::getPOSTData(): remove dead override — AbstractClient
already delegates to \$this->socketConfig->getPOSTData(), which
resolves to IBS\SocketConfig via polymorphism; the override was
byte-for-byte identical to the base and added nothing
- ClientFactory::getClient(): widen \$logger parameter from ?CNR\Logger
to ?LoggerInterface so callers can pass any logger implementation
without a TypeError, consistent with the multi-registrar reality
BREAKING CHANGE: IBS\SessionClient no longer defines login(), logout(),
saveSession(), or reuseSession(). Previously these methods threw
\Exception("Method not supported") — callers handling the unsupported
case via try/catch will now receive a fatal "Call to undefined method"
Error instead. Guard with instanceof CNR\SessionClient before calling
session methods if your code supports multiple registrar types.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7b34f38 to
46eaacd
Compare
…API now after system shutdown
…version number now
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira: RSRMID-2790
Summary
A series of OOP modernization commits that restructure the client hierarchy, extract shared transport logic, fix an LSP violation in
IBS\\SessionClient, and clean up several smaller code-quality issues.Architecture changes
AbstractClientintroduced (CNICnamespace): consolidates all registrar-agnostic logic (config loading, cURL transport setup, credentials, OTE/LIVE switching, IDN conversion, debug mode, proxy/referer, user agent, settings/isOTE accessors). Three abstract methods define the contract every concrete client must satisfy:request(),newSocketConfig(),setDefaultLogger().HttpTransportintroduced (CNICnamespace): owns the cURL handle lifecycle and exposes a singlepost()method. The$chandleproperty has been removed from the client layer entirely;close()now delegates to the transport. Class isfinal(not public API, not extended).CNR\\Clientnow extendsAbstractClientand retains only CNR-specific logic:request()with CNR command formatting,requestNextResponsePage(), andrequestAllResponsePages().IBS\\Clientnow extendsAbstractClientdirectly instead ofCNR\\Client, correctly reflecting that IBS shares the common foundation but not the CNR-specific layer.Session handling
SessionCapabletrait extracted intoCNIC\\CNR: holds the real session implementations (login,logout,saveSession,reuseSession).CNR\\SessionClientuses the trait.IBS\\SessionClientno longer carries misleading stubs that unconditionally threw"Method not supported"— it simply does not include the trait.MONIKER\\SessionClientis unaffected (also does not support sessions).Smaller improvements
$settingsand$isOTEmadeprotectedwith publicgetSettings(): arrayandisOTE(): boolaccessors (previously public, allowing unintended external mutation).ClientFactory::getClient()$loggerparameter widened from?CNR\\Loggerto?LoggerInterfaceso any logger implementation can be passed without aTypeError.IBS\\Client::getPOSTData()override removed —AbstractClientalready delegates to$this->socketConfig->getPOSTData()via polymorphism; the override was byte-for-byte identical to the base.SessionCapable::saveSession()andreuseSession()given: staticreturn type declarations to satisfy the fluent contract.curl_close()calls removed — deprecated since PHP 8.5;CurlHandleis freed automatically by the GC.ClientFactory;switchreplaced withmatch.ColumnandRecordvalue objects updated to constructor promotion +readonly.$loggerPsalmPropertyNotSetInConstructorsuppression added (always set via abstractsetDefaultLogger()inAbstractClient::__construct()— Psalm cannot trace through abstract method dispatch).Release automation
.releaserc.jsonupdated: thesemantic-release-replace-pluginfile target and the@semantic-release/gitassets were changed fromsrc/CNR/Client.phptosrc/AbstractClient.php, wheregetVersion()now lives after the refactor.