new feature: Add if-match & if-none-match support for reader

[Xuanwo]

Feature Description

OpenDAL supports read_with(path).if_match(etag) but not reader_with(path).if_match(etag), it will be great if we can support it.

Problem and Solution

reader could read diffent range of file, so the etag may need to pass while perform real reading IO. The API should be carefully designed.

Additional Context

No response

Are you willing to contribute to the development of this feature?

• Yes, I am willing to contribute to the development of this feature.

[XmchxUp]

Hi, I am trying to implement it.

[Xuanwo]

Hi, I am trying to implement it.

Thant's great! Since this will affect our public API, would you like to propose an RFC first?

[XmchxUp]

@Xuanwo hi, what do you mean by real reading IO? Because my understanding is that the current service backend already has this parameter, and the etag for multiple range requests is the same value.

    pub fn http_get_request(
        &self,
        path: &str,
        range: BytesRange,
        args: &OpRead,
    ) -> Result<Request<Buffer>> {
        let p = build_rooted_abs_path(&self.root, path);

        let url = format!("{}{}", self.endpoint, percent_encode_path(&p));

        let mut req = Request::get(&url);

        if let Some(if_match) = args.if_match() {
            req = req.header(IF_MATCH, if_match);
        }

[Xuanwo]

Because my understanding is that the current service backend already has this parameter, and the ETag for multiple range requests is the same value.

Hi, I'm referring to the public API of opendal: Operator::reader. When a user calls this API, they receive an opendal::Reader. However, at this point, the underlying requests have not yet been executed. So we don't know of the metadata yet.

[meteorgan]

Range condition is not supported for reader now, should we add it ?

[XmchxUp]

I can currently only think of this method add a new api for Reader.

    pub async fn read_with_etag(
        &self,
        range: impl RangeBounds<u64>,
        etag: Option<&str>,
        if_none_match: bool,
    ) -> Result<Buffer> {

I wonder if there are any other ideas?

[Xuanwo]

Hi, @XmchxUp, I believe we need an approach to allow users to utilize different options simultaneously, including if_match, if_none_match, if_modified_since, and so on. So it doesn't look good to me that have a Reader::read_with_etag.

Maybe we can follow the same patten like Operator::read_with that we can have Reader::read_with, Reader::into_futures_async_read_with and Reader::into_bytes_stream_with.

let r = op.reader_with(path).version("version-id").await?;
let bs = r.read_with(0..1024).if_match("etag").if_modified_since(time).await?;

[meteorgan]

Hi, @XmchxUp . I need this feature to address #5115. so maybe we can collaborate on it. How about I propose an RFC first ? @Xuanwo

[Xuanwo]

How about I propose an RFC first ? @Xuanwo

I will first listen to @XmchxUp's intentions. It would be better if he wants to propose an RFC first.

[XmchxUp]

Of course, you can help complete this RFC, my understanding may not be very clear.

Thank you so much @Xuanwo @meteorgan .

[meteorgan]

Of course, you can help complete this RFC, my understanding may not be very clear.

I'll submit a draft RFC in the next few days, and then we can complete it together.

[meteorgan]

Range condition is not supported for reader now, should we add it ?

emmm, According to the design of reader, we don't need the Range condition for reader_with. Instead, we can handle the Range when actually reading the data.

opendal/core/src/types/read/reader.rs

Lines 112 to 115 in 52c96bb

	pub async fn read(&self, range: impl RangeBounds<u64>) -> Result<Buffer> {
	let bufs: Vec<_> = self.clone().into_stream(range).await?.try_collect().await?;
	Ok(bufs.into_iter().flatten().collect())
	}