Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data and file upload support #43

Merged
merged 55 commits into from
Sep 6, 2023
Merged

Data and file upload support #43

merged 55 commits into from
Sep 6, 2023

Conversation

tonyskansf
Copy link
Contributor

This pull request introduces an upload feature allowing clients to upload data and files (no stream or multipart file support yet). The implementation follows the existing download feature design with slight deviation whilst aiming to keep consistency and familiarity with the API. With that, the changes also include an example that showcases the usage of the API.

Please, do not consider this implementation as final but rather as the first draft.

Video

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-06-13.at.17.24.34.mp4

Implementation

UploadTask

The central component is the UploadTask object that encapsulates the ongoing URLSessionUploadTask and provides the task's state updates. Each upload request creates an upload task instance that is internally stored until the request is completed. In case of a failed request, the UploadTask serves as the representation of the failed request and is important in subsequent retry attempts.

UploadTask.State

This object represents the progress of the upload request and is updated continuously by the internal publisher. It is then available to the client as an asynchronous sequence. Since the implementation creates the URLSessionUploadTask with completion handlers, the response is accessible only upon the task's completion. Consequently, the response or error is delivered within the upload task's state.

Retryable

The UploadTask conforms to the Retryable protocol instead of the UploadAPIManager as is in the DownloadAPIManager. This emphasizes the intention that the task itself is retryable, rather than the manager. Despite potentially deviating from the original design of the protocol, this approach seems to work as well. Each upload task has its retryCounter: Counter that determines whether the task should be retried based on the RetryConfiguration.

Additional Notes

  • The sample does not include a complete router example intentionally. I've used this service for testing purposes.

@tonyskansf tonyskansf requested a review from cejanen June 13, 2023 10:30
@tonyskansf tonyskansf linked an issue Jun 14, 2023 that may be closed by this pull request
@gajddo00
Copy link
Contributor

@cejanen What I mean was just different approaches to the same thing which is absolutely fine, but most of the functionality is the same so I was thinking whether it would be feasible to have one generic solution for both that could be inherited from or something in that sense.
Because the differences come down to

  • urlSession.download vs urlSession.upload
  • UrlSessionDownloadTask vs URLSessionUploadTask
  • different progress state object emitted in the stream
    🤔 but I'm just thinking out loud here, maybe it wouldn't even be possible

@tonyskansf
Copy link
Contributor Author

Some things (objects) could certainly be unified, but I'd rather not decide this as it changes the implementation of either of the managers.

@brenovaladao
Copy link

got an error when uploading a specific photo from the simulator

Error Domain=NSItemProviderErrorDomain Code=-1000 "Cannot load representation of type public.jpeg" UserInfo={NSLocalizedDescription=Cannot load representation of type public.jpeg, NSUnderlyingError=0x6000036ba9a0 {Error Domain=NSCocoaErrorDomain Code=4101 "Couldn’t communicate with a helper application." UserInfo={NSUnderlyingError=0x6000036bdda0 {Error Domain=PHAssetExportRequestErrorDomain Code=0 "The operation couldn’t be completed. (PHAssetExportRequestErrorDomain error 0.)" UserInfo={NSLocalizedDescription=The operation couldn’t be completed. (PHAssetExportRequestErrorDomain error 0.), NSUnderlyingError=0x6000036bf660 {Error Domain=PAMediaConversionServiceErrorDomain Code=2 "The operation couldn’t be completed. (PAMediaConversionServiceErrorDomain error 2.)" UserInfo=0x6000038e1820 (not displayed)}}}}}}

@gajddo00
Copy link
Contributor

gajddo00 commented Jul 4, 2023

I also got this, but it's the simulator problem with corrupted images, nothing to do with uploading I think

Copy link
Collaborator

@cejanen cejanen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonyskansf I answered last comments or resolved issues, we're almost done 👍

@cejanen cejanen self-requested a review September 6, 2023 11:19
Copy link
Collaborator

@cejanen cejanen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥡

@cejanen cejanen merged commit 13a5c93 into dev Sep 6, 2023
@cejanen cejanen deleted the feat/upload branch September 6, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplistic upload
4 participants