fix(services/yandex-disk): end the listing when a response has no _embedded - #8072
Conversation
…bedded
next_page handled a 200 like this:
if let Some(embedded) = resp.embedded {
...
return Ok(());
}
// no else
...
Ok(())
A response without _embedded therefore fell all the way out with
ctx.done still false and nothing pushed. That is precisely
PageLister::next's loop condition:
loop {
if let Some(entry) = self.ctx.entries.pop_front() { return Ok(Some(entry)); }
if self.ctx.done { return Ok(None); }
self.inner.next_page(&mut self.ctx).await?;
}
so the same HTTPS request was re-issued for ever -- a hang plus
unbounded request volume against the user's Yandex quota.
_embedded is what a folder carries; a file has none, which is why the
field is Option<Embedded> on MetainformationResponse. And listing a file
path is a documented contract, not a misuse: core/tests/behavior/
async_list.rs test_list_prefix writes a file and lists that exact path,
expecting one FILE entry.
So the no-_embedded branch now emits the file the response describes and
ends the listing. The response is that file's own metainformation, and
parse_info already accepts it -- it is the same type as the items in
_embedded.
The entry construction is shared between the two branches as
build_entry, and the page handling moves into consume_page so it can be
driven without an HTTP round trip. yandex-disk has no directory under
.github/services, so nothing exercised any of this.
Three unit tests. Restoring the fall-through fails the file case and
leaves both folder cases green, which is the shape of the bug.
Matching the shape asked for on apache#8068, apache#8069 and apache#8071 rather than waiting to be asked again: the two extracted helpers go and the branch is written inline, leaving the existing folder loop untouched. resp becomes mut so embedded can be taken out of it, which keeps the rest of the response available for the file case below.
|
Slimmed this to match the shape you asked for on #8068, #8069 and #8071, rather than making you say it again. Both extracted helpers are gone; the no- The fix is unchanged: a 200 with no The removed tests drove |
| return Ok(()); | ||
| } | ||
|
|
||
| // A folder answers with `_embedded`; a file has none, and the response is that |
There was a problem hiding this comment.
Cut. The why is in the commit message instead.
Which issue does this PR close?
None — found while reading the service.
Rationale for this change
next_pagehandled a200like this:A response without
_embeddedfalls all the way out withctx.donefalse and nothing pushed. That is exactlyPageLister::next's loop condition:so the same HTTPS request is re-issued for ever — a hang, plus unbounded request volume against the user's Yandex quota.
_embeddedis what a folder carries. A file has none, which is why the field isOption<Embedded>:And listing a file path is a documented contract rather than a misuse —
core/tests/behavior/async_list.rs:What changes are included in this PR?
The no-
_embeddedbranch now emits the file the response describes and ends the listing. The response is that file's own metainformation, andparse_infoalready accepts it — it is the same type as the items inside_embedded.The entry construction is shared between the two branches as
build_entry, and the page handling moves intoconsume_pageso it can be driven without an HTTP round trip.There is no directory under
.github/servicesfor yandex-disk, so nothing exercised any of this.If you would rather keep the change minimal,
ctx.done = truealone stops the loop — but it leavesliston a file returning nothing, which is whattest_list_prefixforbids.Tests
_embedded)main)cargo test -p opendal-service-yandex-disk --lib→ 5 passed.cargo fmt --all -- --checkandcargo clippy -p opendal-service-yandex-disk --all-targetsboth clean.Are there any user-facing changes?
Yes —
liston a yandex-disk path that is a file returns that file as a single entry and terminates, instead of looping on the same request indefinitely. Folder listings are unchanged.