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.

Sign up for GitHub

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

Jump to bottom

Fix chunk uploads #993

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix chunk uploads #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b7940b
fix(kotlin): chunk upload
byawitz Oct 2, 2024
9c21014
fix(android): chunk upload
byawitz Oct 2, 2024
27d1ade
fix(ruby): chunk upload
byawitz Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 34 additions & 28 deletions templates/android/library/src/main/java/io/package/Client.kt.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Client @JvmOverloads constructor(
internal lateinit var http: OkHttpClient

internal val headers: MutableMap<String, String>

val config: MutableMap<String, String>

internal val cookieJar = ListenableCookieJar(CookieManager(
Expand All @@ -87,14 +87,14 @@ class Client @JvmOverloads constructor(
"x-sdk-platform" to "{{ sdk.platform }}",
"x-sdk-language" to "{{ language.name | caseLower }}",
"x-sdk-version" to "{{ sdk.version }}"{% if spec.global.defaultHeaders | length > 0 %},{% endif %}

{% for key,header in spec.global.defaultHeaders %}
"{{ key | caseLower }}" to "{{ header }}"{% if not loop.last %},{% endif %}
{% endfor %}

)
config = mutableMapOf()

setSelfSigned(selfSigned)
}

Expand All @@ -119,10 +119,10 @@ class Client @JvmOverloads constructor(
{% endfor %}
/**
* Set self Signed
*
*
* @param status
*
* @return this
* @return this
*/
fun setSelfSigned(status: Boolean): Client {
selfSigned = status
Expand Down Expand Up @@ -171,10 +171,10 @@ class Client @JvmOverloads constructor(

/**
* Set endpoint and realtime endpoint.
*
*
* @param endpoint
*
* @return this
* @return this
*/
fun setEndpoint(endpoint: String): Client {
this.endpoint = endpoint
Expand All @@ -200,11 +200,11 @@ class Client @JvmOverloads constructor(

/**
* Add Header
*
*
* @param key
* @param value
*
* @return this
* @return this
*/
fun addHeader(key: String, value: String): Client {
headers[key] = value
Expand All @@ -213,19 +213,19 @@ class Client @JvmOverloads constructor(

/**
* Send the HTTP request
*
*
* @param method
* @param path
* @param headers
* @param params
*
* @return [T]
* @return [T]
*/
@Throws({{ spec.title | caseUcfirst }}Exception::class)
suspend fun <T> call(
method: String,
path: String,
headers: Map<String, String> = mapOf(),
method: String,
path: String,
headers: Map<String, String> = mapOf(),
params: Map<String, Any?> = mapOf(),
responseType: Class<T>,
converter: ((Any) -> T)? = null
Expand Down Expand Up @@ -364,16 +364,22 @@ class Client @JvmOverloads constructor(
var result: Map<*, *>? = null

if (idParamName?.isNotEmpty() == true && params[idParamName] != "unique()") {
// Make a request to check if a file already exists
val current = call(
method = "GET",
path = "$path/${params[idParamName]}",
headers = headers,
params = emptyMap(),
responseType = Map::class.java,
)
val chunksUploaded = current["chunksUploaded"] as Long
offset = chunksUploaded * CHUNK_SIZE
try {
// Make a request to check if a file already exists
val current = call(
method = "GET",
path = "$path/${params[idParamName]}",
headers = headers,
params = emptyMap(),
responseType = Map::class.java,
)
val chunksUploaded = current["chunksUploaded"] as Long
Copy link
Member

Choose a reason for hiding this comment

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

Is Long type the best choice?

offset = chunksUploaded * CHUNK_SIZE
} catch (e: Exception) {
if (e.message != null && !e.message!!.contains("file could not be found")) {
Copy link
Member

Choose a reason for hiding this comment

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

Agree with Steven, this seems fragile...

throw e
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't call() methods already throw these exceptions, is there any need to catch and throw here?

}
}
}

while (offset < size) {
Expand Down Expand Up @@ -460,14 +466,14 @@ class Client @JvmOverloads constructor(
.charStream()
.buffered()
.use(BufferedReader::readText)

val error = if (response.headers["content-type"]?.contains("application/json") == true) {
val map = body.fromJson<Map<String, Any>>()

{{ spec.title | caseUcfirst }}Exception(
map["message"] as? String ?: "",
map["message"] as? String ?: "",
(map["code"] as Number).toInt(),
map["type"] as? String ?: "",
map["type"] as? String ?: "",
body
)
} else {
Expand Down Expand Up @@ -519,4 +525,4 @@ class Client @JvmOverloads constructor(
}
})
}
}
}
48 changes: 27 additions & 21 deletions templates/kotlin/src/main/kotlin/io/appwrite/Client.kt.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class Client @JvmOverloads constructor(

/**
* Prepare the HTTP request
*
*
* @param method
* @param path
* @param headers
Expand Down Expand Up @@ -267,7 +267,7 @@ class Client @JvmOverloads constructor(
* @param headers
* @param params
*
* @return [T]
* @return [T]
*/
@Throws({{ spec.title | caseUcfirst }}Exception::class)
suspend fun <T> call(
Expand All @@ -290,7 +290,7 @@ class Client @JvmOverloads constructor(
* @param headers
* @param params
*
* @return [T]
* @return [T]
*/
@Throws({{ spec.title | caseUcfirst }}Exception::class)
suspend fun redirect(
Expand Down Expand Up @@ -363,16 +363,22 @@ class Client @JvmOverloads constructor(
var result: Map<*, *>? = null

if (idParamName?.isNotEmpty() == true && params[idParamName] != "unique()") {
// Make a request to check if a file already exists
val current = call(
method = "GET",
path = "$path/${params[idParamName]}",
headers = headers,
params = emptyMap(),
responseType = Map::class.java,
)
val chunksUploaded = current["chunksUploaded"] as Long
offset = chunksUploaded * CHUNK_SIZE
try {
// Make a request to check if a file already exists
val current = call(
method = "GET",
path = "$path/${params[idParamName]}",
headers = headers,
params = emptyMap(),
responseType = Map::class.java,
)
val chunksUploaded = current["chunksUploaded"] as Long
offset = chunksUploaded * CHUNK_SIZE
} catch (e: Exception) {
if (e.message != null && !e.message!!.contains("file could not be found")) {
throw e
}
}
}

while (offset < size) {
Expand Down Expand Up @@ -429,7 +435,7 @@ class Client @JvmOverloads constructor(
return converter(result as Map<String, Any>)
}

/**
/**
* Await Redirect
*
* @param request
Expand All @@ -456,14 +462,14 @@ class Client @JvmOverloads constructor(
.charStream()
.buffered()
.use(BufferedReader::readText)

val error = if (response.headers["content-type"]?.contains("application/json") == true) {
val map = body.fromJson<Map<String, Any>>()

{{ spec.title | caseUcfirst }}Exception(
map["message"] as? String ?: "",
map["message"] as? String ?: "",
(map["code"] as Number).toInt(),
map["type"] as? String ?: "",
map["type"] as? String ?: "",
body
)
} else {
Expand Down Expand Up @@ -507,14 +513,14 @@ class Client @JvmOverloads constructor(
.charStream()
.buffered()
.use(BufferedReader::readText)

val error = if (response.headers["content-type"]?.contains("application/json") == true) {
val map = body.fromJson<Map<String, Any>>()

{{ spec.title | caseUcfirst }}Exception(
map["message"] as? String ?: "",
map["message"] as? String ?: "",
(map["code"] as Number).toInt(),
map["type"] as? String ?: "",
map["type"] as? String ?: "",
body
)
} else {
Expand Down Expand Up @@ -564,4 +570,4 @@ class Client @JvmOverloads constructor(
}
})
}
}
}
36 changes: 20 additions & 16 deletions templates/ruby/lib/container/client.rb.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module {{ spec.title | caseUcfirst }}
'x-sdk-platform'=> '{{ sdk.platform }}',
'x-sdk-language'=> '{{ language.name | caseLower }}',
'x-sdk-version'=> '{{ sdk.version }}'{% if spec.global.defaultHeaders | length > 0 %},{% endif %}

{% for key,header in spec.global.defaultHeaders %}
'{{key}}' => '{{header}}'{% if not loop.last %},{% endif %}
{% endfor %}
Expand Down Expand Up @@ -49,7 +49,7 @@ module {{ spec.title | caseUcfirst }}
# @return [self]
def set_endpoint(endpoint)
@endpoint = endpoint

self
end

Expand Down Expand Up @@ -133,15 +133,19 @@ module {{ spec.title | caseUcfirst }}
offset = 0
id_param_name = id_param_name.to_sym if id_param_name
if id_param_name&.empty? == false && params[id_param_name] != "unique()"
begin
# Make a request to check if a file already exists
current = call(
method: "GET",
path: "#{path}/#{params[id_param_name]}",
headers: headers,
params: {}
)
chunks_uploaded = current['chunksUploaded'].to_i
offset = chunks_uploaded * @chunk_size
current = call(
method: "GET",
path: "#{path}/#{params[id_param_name]}",
headers: headers,
params: {}
)
chunks_uploaded = current['chunksUploaded'].to_i
offset = chunks_uploaded * @chunk_size
rescue => error
raise {{spec.title | caseUcfirst}}::Exception.new(error.message) unless error.message.downcase.include?("file could not be found")
end
end

while offset < size
Expand Down Expand Up @@ -202,7 +206,7 @@ module {{ spec.title | caseUcfirst }}
@http = Net::HTTP.new(uri.host, uri.port) unless defined? @http
@http.use_ssl = !@self_signed
payload = ''

headers = @headers.merge(headers)

params.compact!
Expand Down Expand Up @@ -237,7 +241,7 @@ module {{ spec.title | caseUcfirst }}
if response_type == "location"
return location
end

# Handle Redirects
if (response.class == Net::HTTPRedirection || response.class == Net::HTTPMovedPermanently)
uri = URI.parse(uri.scheme + "://" + uri.host + "" + location)
Expand Down Expand Up @@ -272,7 +276,7 @@ module {{ spec.title | caseUcfirst }}

return response
end

def encode_form_data(value, key=nil)
case value
when Hash
Expand Down Expand Up @@ -304,13 +308,13 @@ module {{ spec.title | caseUcfirst }}
when Hash then value.map { |k,v| encode(v, append_key(key,k)) }.join('&')
when Array then value.map { |v| encode(v, "#{key}[]") }.join('&')
when nil then ''
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but can we fix the spacing here? Looks weird

else
"#{key}=#{CGI.escape(value.to_s)}"
else
"#{key}=#{CGI.escape(value.to_s)}"
end
end

def append_key(root_key, key)
root_key.nil? ? key : "#{root_key}[#{key.to_s}]"
end
end
end
end