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

[FileUpload] allow to sub class FileUpload #2569

Merged
merged 5 commits into from
Sep 21, 2020

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Sep 10, 2020

This allows callers to use the okio.Source they want to upload files. With Android11 removing filestorage access, this will become mandatory.

Also took this opportunity to move some files to Kotlin.

fixes #2392

@martinbonnin martinbonnin changed the base branch from master to bug-2409/send-null-in-file-upload September 10, 2020 17:31
@@ -1,19 +1,37 @@
package com.apollographql.apollo.api

class FileUpload(val mimetype: String, val filePath: String) {
override fun equals(other: Any?): Boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't find a need to implement equals/hashCode on this class. Am I missing something?

@martinbonnin
Copy link
Contributor Author

@darjke can you take a look and let me know if it works for you? I didn't explicitely use contentUri but the FileUpload now has a writeTo(sink: BufferedSink) so that implementation can decide where the data comes from. I feel it's more flexible and also it doesn't pull an Android dependency. Let me know your thoughts!

* The fileName to send to the server. Might be null
*/
open fun fileName(): String? {
throw UnsupportedOperationException("ApolloGraphQL: if you're not passing a `filePath` parameter, you must override `FileUpload.fileName`")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove filePath field and make this class abstract? Also make filename to be non null abstrac property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be much nicer but also a breaking change which I'd like to avoid if possible. I'll do it in dev-3x but given Android 11 is just out, I think it makes sense to release this before 3.0.0.

Base automatically changed from bug-2409/send-null-in-file-upload to main September 17, 2020 07:08
@martinbonnin martinbonnin merged commit 342996b into main Sep 21, 2020
@martinbonnin martinbonnin deleted the feature-2392/source-file-upload branch September 21, 2020 14:02
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.

Upload file with using content uri
2 participants