-
Notifications
You must be signed in to change notification settings - Fork 193
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
Fix paginator bug where None
was returned immediately
#1083
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,7 +9,6 @@ import software.amazon.smithy.model.Model | |||||
import software.amazon.smithy.model.knowledge.PaginatedIndex | ||||||
import software.amazon.smithy.model.shapes.OperationShape | ||||||
import software.amazon.smithy.model.shapes.ServiceShape | ||||||
import software.amazon.smithy.model.shapes.StringShape | ||||||
import software.amazon.smithy.model.traits.IdempotencyTokenTrait | ||||||
import software.amazon.smithy.model.traits.PaginatedTrait | ||||||
import software.amazon.smithy.rust.codegen.rustlang.CargoDependency | ||||||
|
@@ -175,12 +174,13 @@ class PaginatorGenerator private constructor( | |||||
let done = match resp { | ||||||
Ok(ref resp) => { | ||||||
let new_token = #{output_token}(resp); | ||||||
if new_token == input.$inputTokenMember.as_ref() { | ||||||
let is_empty = ${nextTokenEmpty("new_token")}; | ||||||
if !is_empty && new_token == input.$inputTokenMember.as_ref() { | ||||||
let _ = tx.send(Err(#{SdkError}::ConstructionFailure("next token did not change, aborting paginator. This indicates an SDK or AWS service bug.".into()))).await; | ||||||
return; | ||||||
} | ||||||
input.$inputTokenMember = new_token.cloned(); | ||||||
${nextTokenEmpty("input.$inputTokenMember")} | ||||||
is_empty | ||||||
}, | ||||||
Err(_) => true, | ||||||
}; | ||||||
|
@@ -192,7 +192,6 @@ class PaginatorGenerator private constructor( | |||||
return | ||||||
} | ||||||
} | ||||||
|
||||||
})) | ||||||
} | ||||||
} | ||||||
|
@@ -276,12 +275,7 @@ class PaginatorGenerator private constructor( | |||||
} | ||||||
|
||||||
private fun nextTokenEmpty(token: String): String { | ||||||
val tokenType = model.expectShape(paginationInfo.outputTokenMemberPath.last().target) | ||||||
if (tokenType is StringShape) { | ||||||
return "$token.as_deref().unwrap_or_default().is_empty()" | ||||||
} else { | ||||||
return "$token.is_none()" | ||||||
} | ||||||
return "$token.map(|token|token.is_empty()).unwrap_or(true)" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should prefer function pointers in place of closures where possible:
Suggested change
It might not be possible depending on how type inference interprets this though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be either a string or a hashmap |
||||||
} | ||||||
|
||||||
private fun pageSizeSetter() = writable { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly does this mean? How many "kinds" of unset are there? I can think of
a. field returned but empty or set to a value signifying empty
b. no field returned
Were we only covering case a previously and now we're covering case b too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it can either be
<nextToken></nextToken>
, or as is the case here,<nextToken>
is not present in the response at all. In one case, we getSome("")
, in the other, we getNone
.None
used to be handled properly, but regressed when I added the failsafe to avoid infinite loops