Skip to content

The TransportInterface may have incorrect capabilities. #157

@kafkiansky

Description

@kafkiansky

Hi.

Currently, TransportInterface includes two additional methods: downloadFile and downloadFileTo. In my opinion, the transport would only need the downloadFile method. It's not correct to ask the transport to save to a file: first, saving isn't limited to files — it could also be to S3, where we could implement zero-copy if TransportInterface::downloadFile returned a StreamInterface that could be directly sent to S3; second, many transports, including non-blocking ones like amphp/http-client, simply don't support saving to a file, so they would have to duplicate file-saving logic among themselves — logic that could otherwise be abstracted away.

I propose the following interface:

/**
 * @api
 */
interface TransportInterface
{
    /**
     * @psalm-param array<string, mixed> $data
     */
    public function send(
        string $urlPath,
        array $data = [],
        HttpMethod $httpMethod = HttpMethod::POST,
    ): ApiResponse;

    /**
     * Downloads a file by URL.
     *
     * @param string $url The URL of the file to download.
     *
     * @throws DownloadFileException If an error occurred while downloading the file.
     */
    public function downloadFile(string $url): StreamInterface;
}

The file-saving logic doesn’t seem necessary to implement in this library at all.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions