code cleanup

[Fivell]

Fixes a real gap: Export::download()/downloadAndDecompress() built their own curl handle from scratch, bypassing whatever the consumer passed to Configuration::configure($credentials, $httpClientConfig) — proxy, timeout, SSL options, custom middleware all silently didn't apply to downloads (and downloads had no timeout at all, so they could hang indefinitely). Routes both methods through the SDK's shared, already-configured Guzzle client instead, via a new Configuration::getHttpClient() seam, so consumer HTTP config now actually applies everywhere. Not a line-count reduction (net +7 lines — the seam is new code). No behavior changes for the happy path, all tests pass.

[Copilot]

Pull request overview

This PR refactors file upload/download paths to use the SDK’s configured Guzzle client instead of hand-rolled curl calls, and updates tests to assert the outgoing HTTP requests.

Changes:

• Replaced curl-based download logic in Export and SupportingDocumentTemplate with Configuration::getHttpClient()->request(...) and sink.
• Reworked EncryptedFile::upload() to use Guzzle multipart requests and removed the custom multipart builder (and the $boundary parameter).
• Added/updated unit tests to validate download/upload behavior via a configured Guzzle handler stack.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/SupportingDocumentTemplateTest.php	Adds a test asserting SupportingDocumentTemplate::download() uses the configured Guzzle client and writes to disk.
tests/EncryptedFileTest.php	Adds a test asserting multipart upload goes through the configured Guzzle client and adjusts signature expectations.
src/Item/SupportingDocumentTemplate.php	Replaces curl download with Guzzle request() + sink.
src/Item/Export.php	Replaces curl export download with Guzzle; simplifies gzip decompression using stream wrappers.
src/Item/EncryptedFile.php	Replaces manual multipart body assembly with Guzzle multipart; returns UploadResult from response.
src/Configuration.php	Stores the configured Guzzle client for reuse; removes getApiKey() / getBaseUri() helpers.

Suppressed comments (2)

src/Item/EncryptedFile.php:69

• EncryptedFile::upload() no longer handles transport-level failures (DNS/connection timeouts, etc.). Guzzle will throw a GuzzleException in these cases, whereas the previous curl implementation returned false/0 and still produced an UploadResult. To keep behavior consistent and avoid unexpected exceptions for SDK consumers, catch RequestException/GuzzleException and convert it into an UploadResult (using the response if present).

        $response = \Didww\Configuration::getHttpClient()->request('POST', $url, [
            'headers' => [
                'Api-Key' => $apiKey,
                'X-DIDWW-API-Version' => $apiVersion,
                'Accept' => 'application/json',
            ],
            'multipart' => $multipart,
            'http_errors' => false,
        ]);

        return new UploadResult((string) $response->getBody(), $response->getStatusCode());

src/Configuration.php:51

• Configuration removed public getApiKey() and getBaseUri() methods. If these were part of the supported public API, this is a breaking change for SDK users. Consider reintroducing them as @deprecated wrappers to preserve backwards compatibility.

        self::$documentClient = new \Swis\JsonApi\Client\DocumentClient($client, $responseParser);
    }
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/Item/Export.php:132

• The request relies on the Guzzle client's default http_errors setting. If consumers pass ['http_errors' => false] into Configuration::configure(), HTTP 4xx/5xx responses will no longer raise an exception and $error will incorrectly remain null (changing behavior vs the previous CURLOPT_FAILONERROR). Explicitly enable http_errors for this request (and adjust the inline comment accordingly).

                'sink' => $destHandle,
                'allow_redirects' => true,
                // HTTP code >= 400 will throw a GuzzleException, same as CURLOPT_FAILONERROR before.
            ]);

src/Item/Export.php:136

• Only GuzzleException is caught here. Any other throwable from request() (e.g., invalid URL/URI or invalid sink) will bubble up as an exception, which breaks the method’s current “return true|string” error-reporting behavior. Consider catching \Throwable after GuzzleException and returning the message like before.

        } catch (\GuzzleHttp\Exception\GuzzleException $e) {
            $error = $e->getMessage();
        }

src/Configuration.php:26

• getHttpClient() promises to return ClientInterface, but self::$httpClient is uninitialized until configure() runs, so calling this early will currently produce a TypeError (returning null). Consider adding an explicit guard with a clear exception message to make misconfiguration easier to diagnose.

    public static function getHttpClient()
    {
        return self::$httpClient;
    }

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/Item/Export.php:126

• download() implementation switched from cURL to Guzzle streaming, but the existing Export tests only cover the success path. It would be good to add a regression test for a failed HTTP response (e.g., 404/500) to assert that download() returns an error and does not leave an error body written into the destination file.

        try {
            \Didww\Configuration::getHttpClient()->request('GET', $this->getAttributes()['url'], [
                'headers' => [
                    'Api-Key' => $apiKey,
                    'User-Agent' => 'didww-php-sdk/'.\Didww\Client::sdkVersion(),

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Item/Export.php:126

• download() relies on the configured Guzzle client defaults. If a consumer configures the client with http_errors => false, 4xx/5xx responses will no longer fail (curl previously used CURLOPT_FAILONERROR). Also, if the export url attribute is null/empty, Guzzle can throw a non-GuzzleException (e.g., InvalidArgumentException), which currently bypasses the catch and can crash the call. Consider validating the URL and forcing http_errors for this request to preserve the previous behavior.

        try {
            \Didww\Configuration::getHttpClient()->request('GET', $this->getAttributes()['url'], [
                'headers' => [
                    'Api-Key' => $apiKey,
                    'User-Agent' => 'didww-php-sdk/'.\Didww\Client::sdkVersion(),

src/Item/Export.php:138

• The comment about Guzzle's sink closing the underlying resource is misleading here. Since this method accepts either a path or an already-open resource, the intent is: close the handle only when this method opened it, and leave caller-provided resources open.

        // Guzzle's 'sink' stream wrapper already closes the underlying resource once
        // the response body has been written to it.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud