fix(services/cloudflare-kv): report the stored etag for listed directories

[PDGGK]

Which issue does this PR close?

None — found while reading the lister.

Rationale for this change

build_entry_for_item gave every directory entry a fresh build_tmp_path_of string as its etag:

let entry_metadata = if name.ends_with('/') {
    Metadata::new(EntryMode::DIR)
        .with_etag(build_tmp_path_of(&name))   // random, thrown away next call
        .with_content_length(0)
} else {
    Metadata::new(EntryMode::FILE)
        .with_etag(metadata.etag)              // the stored value, right here
        ...

That string is random, so:

• the same unchanged directory reports a different etag on every list call, and
• it never equals the etag stat reads out of the same record — which is also the value stat compares if_match / if_none_match against (backend.rs, stat):

if let Some(if_match) = &args.if_match()
    && if_match != &metadata.etag
{
    return Err(Error::new(ErrorKind::ConditionNotMatch, "etag mismatch"));
}

So taking an etag from a listing and feeding it back — op.stat_with(dir).if_match(etag) — fails with ConditionNotMatch every time, and a caller comparing etags across two listings sees every directory as changed.

The stored value was already in hand: CfKvListKey carries the record's CfKvMetadata, and the file branch three lines below was already reading metadata.etag from it. Directories have a stored etag too — create_dir and the writer both persist one.

What changes are included in this PR?

• The directory branch reports metadata.etag, the same field the file branch uses.
• The one directory entry with no record behind it — the placeholder the non-recursive branch synthesises from the keys underneath when the listed path is not itself a key — now reports no etag rather than a fabricated one.
• build_entry_for_item never used self, so it moves out of the impl and becomes directly testable.

Existing practice in the repo backs both halves: those two lines were the only places that invented an etag for a directory. azfile's lister attaches dir.properties.etag, the value the server returned; every other service leaves the field unset.

Etag generation is untouched — the writer and create_dir still mint the value with build_tmp_path_of, and since that value is persisted with the record it stays stable for the life of the entry.

Tests

Three unit tests. Reverting only the directory branch to the previous expression fails exactly two of them, and the failure output is the bug:

---- lister::tests::dir_entry_etag_is_the_same_on_every_list_call ----
assertion `left == right` failed
  left: Some("sub/.AvaaBbxz")
 right: Some("sub/.xHzwzn53")

• dir_entry_reports_the_stored_etag — pins the stored value.
• dir_entry_etag_is_the_same_on_every_list_call — only asserts two listings agree, so it holds whatever scheme the service picks for the etag.
• file_entry_reports_the_stored_etag — passes either way, so the first two are not trivially red.

The synthesised placeholder is not unit-tested: reaching it needs a live lister and a page context, so that one line rests on the argument above rather than on a test.

cargo test -p opendal-service-cloudflare-kv --lib → 8 passed. cargo fmt --all -- --check and cargo clippy -p opendal-service-cloudflare-kv --all-targets both clean.

Are there any user-facing changes?

Yes, for services-cloudflare-kv: a listed directory now carries the etag stored with it instead of a random string, and the synthesised directory placeholder carries none. Both make listed metadata agree with stat.

[PDGGK]

Slimmed this to match the shape you asked for on #8068, #8069 and #8071, rather than making you say it a fourth time.

build_entry_for_item goes back to being a method — it was one before this PR; I had only moved it out because it never touches self. The three tests added with it are gone. The diff is now the two etag lines, +4/-4.

The relative_to_root tests already in the file are from #8048 and are untouched.

What the removed tests covered, so it is a decision rather than an omission: that a listed directory reports the same etag on two successive calls. The old value came from build_tmp_path_of, so it was random per call and could never equal the one stat compares if_match against. Not asking to keep them — just noting what is no longer pinned.

[erickguan]

Can you include a Cloudflare response from the wire in the test?

[PDGGK]

Added in 6edf1ec — a list keys response goes through CfKvListResponse rather than a hand-built struct, so the deserialization is part of what is exercised:

{
    "success": true,
    "result": [
        { "name": "data/sub/",   "metadata": { "etag": "sub/.AvaaBbxz",   "last_modified": "...", "content_length": 0, "is_dir": true  } },
        { "name": "data/a.txt", "metadata": { "etag": "a.txt.xHzwzn53", "last_modified": "...", "content_length": 7, "is_dir": false } }
    ],
    "result_info": { "cursor": "" }
}

It asserts both entries, which is the premise the change rests on: a directory key's metadata blob carries the same CfKvMetadata a file's does, etag included, because this service wrote it itself through set. The etag values are shaped the way build_tmp_path_of mints them at write time, so the fixture matches what a real namespace would hold.

Restoring the old expression fails this test and nothing else — 5 passed, 1 failed.

I had stripped the tests from this PR preemptively after your notes on #8068 and #8069; this is a better test than the ones I removed, so thanks for asking for it rather than letting it go.

[erickguan]

Where did you get this payload? How did you get it?

[PDGGK]

Straight answer: I did not capture it. I have no Cloudflare account and never saw a live response. I reconstructed it from this repository, and my comment saying "as the Cloudflare API returns it" overstated that — corrected in the push just now to say plainly that it is reconstructed.

What each half is actually derived from:

The metadata blob is not Cloudflare's — it is ours. CloudflareWriter::write_once builds it and CloudflareKvCore::set sends it as a form part:

let cf_kv_metadata = CfKvMetadata {
    etag: build_tmp_path_of(&self.path),
    last_modified: Timestamp::now().to_string(),
    content_length: bs.len(),
    is_dir: self.path.ends_with('/'),
};
...
FormDataPart::new("metadata")
    .content(serde_json::to_string(&metadata)?)

So etag / last_modified / content_length / is_dir are exactly what serde emits for CfKvMetadata, and the etag values are shaped the way build_tmp_path_of mints them. That half is checkable against the source rather than taken on trust.

The envelope — success, result, result_info — I wrote to match what CfKvListResponse deserializes. Extra fields Cloudflare sends are ignored by serde, and fields it does not send would show up as a parse failure, so the test would not silently pass on a wrong shape. But I cannot claim the envelope is byte-faithful, only that it is a superset-compatible subset.

If that is not good enough, three options and I am happy with any of them:

1. Keep it, with the corrected comment making the provenance explicit.
2. Replace it with a real capture if you or someone with a namespace can paste one — I would rather use a genuine payload.
3. Drop the JSON and go back to constructing CfKvListKey directly, which tests the entry logic without pretending to test the wire format.

Tell me which and I will push it.