You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "realtaobo (via GitHub)" <gi...@apache.org> on 2023/10/09 14:36:39 UTC

[PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

realtaobo opened a new pull request, #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248

   for #3216 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "realtaobo (via GitHub)" <gi...@apache.org>.
realtaobo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1356905193


##########
core/src/services/d1/error.rs:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use http::Response;
+use http::StatusCode;
+
+use crate::raw::*;
+use crate::Error;
+use crate::ErrorKind;
+use crate::Result;
+
+/// Parse error response into Error.
+pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> {
+    let (parts, body) = resp.into_parts();
+    let bs = body.bytes().await?;
+
+    let (kind, retryable) = match parts.status {

Review Comment:
   Yes, but when sql parm error, it return 500 and message in response. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "realtaobo (via GitHub)" <gi...@apache.org>.
realtaobo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1357026805


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;
+        let body_bytes = body_string.as_bytes().to_owned();
+        req.body(AsyncBody::Bytes(body_bytes.into()))
+            .map_err(new_request_build_error)
+    }
+}
+
+#[async_trait]
+impl kv::Adapter for Adapter {
+    fn metadata(&self) -> kv::Metadata {
+        kv::Metadata::new(
+            Scheme::D1,
+            &self.table,
+            Capability {
+                read: true,
+                write: true,
+                ..Default::default()
+            },
+        )
+    }
+
+    async fn get(&self, path: &str) -> Result<Option<Vec<u8>>> {
+        let query = format!(
+            "SELECT {} FROM {} WHERE {} = ? LIMIT 1",
+            self.value_field, self.table, self.key_field
+        );
+        let req = self.create_d1_query_request(&query, vec![path.into()])?;
+
+        let resp = self.client.send(req).await?;
+        let status = resp.status();
+        match status {
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                let body = resp.into_body().bytes().await?;

Review Comment:
   @Xuanwo I've made updates to this code based on my understanding, but I still have some questions. Could you please provide me with some guidance?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1357078540


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;
+        let body_bytes = body_string.as_bytes().to_owned();
+        req.body(AsyncBody::Bytes(body_bytes.into()))
+            .map_err(new_request_build_error)
+    }
+}
+
+#[async_trait]
+impl kv::Adapter for Adapter {
+    fn metadata(&self) -> kv::Metadata {
+        kv::Metadata::new(
+            Scheme::D1,
+            &self.table,
+            Capability {
+                read: true,
+                write: true,
+                ..Default::default()
+            },
+        )
+    }
+
+    async fn get(&self, path: &str) -> Result<Option<Vec<u8>>> {
+        let query = format!(
+            "SELECT {} FROM {} WHERE {} = ? LIMIT 1",
+            self.value_field, self.table, self.key_field
+        );
+        let req = self.create_d1_query_request(&query, vec![path.into()])?;
+
+        let resp = self.client.send(req).await?;
+        let status = resp.status();
+        match status {
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                let body = resp.into_body().bytes().await?;

Review Comment:
   > And `in a strong and explicit manner` means?
   
   Implement a `D1QueryResponse` and dervice `Deserialize` for with instead of playing with `serde_json::Map` and `serde_json::Value`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#issuecomment-1753247668

   you can run the test via `cargo test services_d1 --features services-d1`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1356930077


##########
core/src/services/d1/error.rs:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use http::Response;
+use http::StatusCode;
+
+use crate::raw::*;
+use crate::Error;
+use crate::ErrorKind;
+use crate::Result;
+
+/// Parse error response into Error.
+pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> {
+    let (parts, body) = resp.into_parts();
+    let bs = body.bytes().await?;
+
+    let (kind, retryable) = match parts.status {

Review Comment:
   > Yes, but when sql parm error, it return 500 and message in response.
   
   So it's better for us to parse the error response correctly. Visit service s3 as an example.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "realtaobo (via GitHub)" <gi...@apache.org>.
realtaobo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1357026805


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;
+        let body_bytes = body_string.as_bytes().to_owned();
+        req.body(AsyncBody::Bytes(body_bytes.into()))
+            .map_err(new_request_build_error)
+    }
+}
+
+#[async_trait]
+impl kv::Adapter for Adapter {
+    fn metadata(&self) -> kv::Metadata {
+        kv::Metadata::new(
+            Scheme::D1,
+            &self.table,
+            Capability {
+                read: true,
+                write: true,
+                ..Default::default()
+            },
+        )
+    }
+
+    async fn get(&self, path: &str) -> Result<Option<Vec<u8>>> {
+        let query = format!(
+            "SELECT {} FROM {} WHERE {} = ? LIMIT 1",
+            self.value_field, self.table, self.key_field
+        );
+        let req = self.create_d1_query_request(&query, vec![path.into()])?;
+
+        let resp = self.client.send(req).await?;
+        let status = resp.status();
+        match status {
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                let body = resp.into_body().bytes().await?;

Review Comment:
   @Xuanwo I've made updates to this code based on my understanding, but I still have some questions. Could you please provide me with some suggestion? And `in a strong and explicit manner` means?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "realtaobo (via GitHub)" <gi...@apache.org>.
realtaobo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1357792406


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;
+        let body_bytes = body_string.as_bytes().to_owned();
+        req.body(AsyncBody::Bytes(body_bytes.into()))
+            .map_err(new_request_build_error)
+    }
+}
+
+#[async_trait]
+impl kv::Adapter for Adapter {
+    fn metadata(&self) -> kv::Metadata {
+        kv::Metadata::new(
+            Scheme::D1,
+            &self.table,
+            Capability {
+                read: true,
+                write: true,
+                ..Default::default()
+            },
+        )
+    }
+
+    async fn get(&self, path: &str) -> Result<Option<Vec<u8>>> {
+        let query = format!(
+            "SELECT {} FROM {} WHERE {} = ? LIMIT 1",
+            self.value_field, self.table, self.key_field
+        );
+        let req = self.create_d1_query_request(&query, vec![path.into()])?;
+
+        let resp = self.client.send(req).await?;
+        let status = resp.status();
+        match status {
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                let body = resp.into_body().bytes().await?;

Review Comment:
   > Implement a `D1QueryResponse` and dervice `Deserialize` for with instead of playing with `serde_json::Map` and `serde_json::Value`.
   I'll mark this as resolved, can you help review the latest code?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1356930077


##########
core/src/services/d1/error.rs:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use http::Response;
+use http::StatusCode;
+
+use crate::raw::*;
+use crate::Error;
+use crate::ErrorKind;
+use crate::Result;
+
+/// Parse error response into Error.
+pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> {
+    let (parts, body) = resp.into_parts();
+    let bs = body.bytes().await?;
+
+    let (kind, retryable) = match parts.status {

Review Comment:
   > Yes, but when sql parm error, it return 500 and message in response.
   
   So it's better for us to parse the error response correctly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#issuecomment-1761062928

   Mostly LGTM! The only thing left here is to make clippy happy. Please make sure you have rust updated to latest via `rustup update`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1356624106


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,

Review Comment:
   How about using `account_id`?



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {

Review Comment:
   We don't need to specify the endpoint. There is only one instance of cloudflare.



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,

Review Comment:
   The same, how about using `database_id`?



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"

Review Comment:
   We should return an error instead of use `account-identifier` as default.



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {

Review Comment:
   the same.



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.

Review Comment:
   Return error if table not specified.



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;

Review Comment:
   Please use `serde_json::to_vec` to avoid unexpected clone.



##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,376 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+use serde_json::{de, Value};
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+use super::model::D1Response;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    token: Option<String>,
+    account_identifier: Option<String>,
+    database_identifier: Option<String>,
+
+    endpoint: Option<String>,
+    http_client: Option<HttpClient>,
+    root: Option<String>,
+
+    table: Option<String>,
+    key_field: Option<String>,
+    value_field: Option<String>,
+}
+
+impl Debug for D1Builder {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Builder");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("root", &self.root);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl D1Builder {
+    /// Set api token for the cloudflare d1 service.
+    ///
+    /// create a api token from [here](https://dash.cloudflare.com/profile/api-tokens)
+    pub fn token(&mut self, token: &str) -> &mut Self {
+        if !token.is_empty() {
+            self.token = Some(token.to_string());
+        }
+        self
+    }
+
+    /// Set the account identifier for the cloudflare d1 service.
+    ///
+    /// get the account identifier from Workers & Pages -> Overview -> Account ID
+    /// default: "account-identifier"
+    pub fn account_identifier(&mut self, account_identifier: &str) -> &mut Self {
+        if !account_identifier.is_empty() {
+            self.account_identifier = Some(account_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set the database identifier for the cloudflare d1 service.
+    ///
+    /// get the database identifier from Workers & Pages -> D1 -> [Your Database] -> Database ID
+    /// default: "database-identifier"
+    pub fn database_identifier(&mut self, database_identifier: &str) -> &mut Self {
+        if !database_identifier.is_empty() {
+            self.database_identifier = Some(database_identifier.to_string());
+        }
+        self
+    }
+
+    /// Set endpoint for http backend.
+    ///
+    /// For more information, please refer to [D1 Database API](https://developers.cloudflare.com/api/operations/cloudflare-d1-query-database)
+    /// default: "https://api.cloudflare.com/client/v4"
+    pub fn endpoint(&mut self, v: &str) -> &mut Self {
+        if !v.is_empty() {
+            self.endpoint = Some(v.to_string());
+        }
+        self
+    }
+
+    /// set the working directory, all operations will be performed under it.
+    ///
+    /// default: "/"
+    pub fn root(&mut self, root: &str) -> &mut Self {
+        if !root.is_empty() {
+            self.root = Some(root.to_owned());
+        }
+        self
+    }
+
+    /// Set the table name of the d1 service to read/write.
+    ///
+    /// Default to `kv` if not specified.
+    pub fn table(&mut self, table: &str) -> &mut Self {
+        if !table.is_empty() {
+            self.table = Some(table.to_owned());
+        }
+        self
+    }
+
+    /// Set the key field name of the d1 service to read/write.
+    ///
+    /// Default to `key` if not specified.
+    pub fn key_field(&mut self, key_field: &str) -> &mut Self {
+        if !key_field.is_empty() {
+            self.key_field = Some(key_field.to_string());
+        }
+        self
+    }
+
+    /// Set the value field name of the d1 service to read/write.
+    ///
+    /// Default to `value` if not specified.
+    pub fn value_field(&mut self, value_field: &str) -> &mut Self {
+        if !value_field.is_empty() {
+            self.value_field = Some(value_field.to_string());
+        }
+        self
+    }
+}
+
+impl Builder for D1Builder {
+    const SCHEME: Scheme = Scheme::D1;
+    type Accessor = D1Backend;
+
+    fn from_map(map: HashMap<String, String>) -> Self {
+        let mut builder = D1Builder::default();
+        map.get("token").map(|v| builder.token(v));
+        map.get("account_identifier")
+            .map(|v| builder.account_identifier(v));
+        map.get("database_identifier")
+            .map(|v| builder.database_identifier(v));
+
+        map.get("endpoint").map(|v| builder.endpoint(v));
+        map.get("root").map(|v| builder.root(v));
+
+        map.get("table").map(|v| builder.table(v));
+        map.get("key_field").map(|v| builder.key_field(v));
+        map.get("value_field").map(|v| builder.value_field(v));
+        builder
+    }
+
+    fn build(&mut self) -> Result<Self::Accessor> {
+        let mut authorization = None;
+        if let Some(token) = &self.token {
+            authorization = Some(format_authorization_by_bearer(token)?)
+        }
+
+        let account_identifier = self
+            .account_identifier
+            .clone()
+            .unwrap_or_else(|| "account-identifier".to_string());
+
+        let database_identifier = self
+            .database_identifier
+            .clone()
+            .unwrap_or_else(|| "database-identifier".to_string());
+
+        let endpoint = self
+            .endpoint
+            .clone()
+            .unwrap_or_else(|| "https://api.cloudflare.com/client/v4".to_string());
+
+        let client = if let Some(client) = self.http_client.take() {
+            client
+        } else {
+            HttpClient::new().map_err(|err| {
+                err.with_operation("Builder::build")
+                    .with_context("service", Scheme::D1)
+            })?
+        };
+
+        let table = self.table.clone().unwrap_or_else(|| "kv".to_string());
+
+        let key_field = self.key_field.clone().unwrap_or_else(|| "key".to_string());
+
+        let value_field = self
+            .value_field
+            .clone()
+            .unwrap_or_else(|| "value".to_string());
+
+        let root = normalize_root(
+            self.root
+                .clone()
+                .unwrap_or_else(|| "/".to_string())
+                .as_str(),
+        );
+        Ok(D1Backend::new(Adapter {
+            authorization,
+            account_identifier,
+            database_identifier,
+            endpoint,
+            client,
+            table,
+            key_field,
+            value_field,
+        })
+        .with_root(&root))
+    }
+}
+
+pub type D1Backend = kv::Backend<Adapter>;
+
+#[derive(Clone)]
+pub struct Adapter {
+    authorization: Option<String>,
+    account_identifier: String,
+    database_identifier: String,
+
+    endpoint: String,
+    client: HttpClient,
+
+    table: String,
+    key_field: String,
+    value_field: String,
+}
+
+impl Debug for Adapter {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let mut ds = f.debug_struct("D1Adapter");
+        ds.field("endpoint", &self.endpoint);
+        ds.field("table", &self.table);
+        ds.field("key_field", &self.key_field);
+        ds.field("value_field", &self.value_field);
+        ds.finish()
+    }
+}
+
+impl Adapter {
+    fn create_d1_query_request(&self, sql: &str, params: Vec<Value>) -> Result<Request<AsyncBody>> {
+        let p = format!(
+            "/accounts/{}/d1/database/{}/query",
+            self.account_identifier, self.database_identifier
+        );
+        let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
+
+        let mut req = Request::post(&url);
+        if let Some(auth) = &self.authorization {
+            req = req.header(header::AUTHORIZATION, auth);
+        }
+        req = req.header(header::CONTENT_TYPE, "application/json");
+
+        let json = serde_json::json!({
+            "sql": sql,
+            "params": params,
+        });
+
+        let body_string = serde_json::to_string(&json).map_err(new_json_serialize_error)?;
+        let body_bytes = body_string.as_bytes().to_owned();
+        req.body(AsyncBody::Bytes(body_bytes.into()))
+            .map_err(new_request_build_error)
+    }
+}
+
+#[async_trait]
+impl kv::Adapter for Adapter {
+    fn metadata(&self) -> kv::Metadata {
+        kv::Metadata::new(
+            Scheme::D1,
+            &self.table,
+            Capability {
+                read: true,
+                write: true,
+                ..Default::default()
+            },
+        )
+    }
+
+    async fn get(&self, path: &str) -> Result<Option<Vec<u8>>> {
+        let query = format!(
+            "SELECT {} FROM {} WHERE {} = ? LIMIT 1",
+            self.value_field, self.table, self.key_field
+        );
+        let req = self.create_d1_query_request(&query, vec![path.into()])?;
+
+        let resp = self.client.send(req).await?;
+        let status = resp.status();
+        match status {
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                let body = resp.into_body().bytes().await?;

Review Comment:
   Please parse the response in a strong and explicit manner. The response for `SELECT {} FROM {} WHERE {} = ? LIMIT 1` should be clear and ensure accuracy.
   
   Return unexpected error if the response is not as expected.



##########
core/src/services/d1/error.rs:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use http::Response;
+use http::StatusCode;
+
+use crate::raw::*;
+use crate::Error;
+use crate::ErrorKind;
+use crate::Result;
+
+/// Parse error response into Error.
+pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> {
+    let (parts, body) = resp.into_parts();
+    let bs = body.bytes().await?;
+
+    let (kind, retryable) = match parts.status {

Review Comment:
   D1 seems not return error this way. We can polish this part in the future.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1350398013


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,263 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    root: Option<String>,
+    endpoint: Option<String>,
+    sql: Option<String>,

Review Comment:
   SQL should be constructed by OpenDAL itself instead of in the config. Maybe we can visit postgresql for reference.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#discussion_r1350398013


##########
core/src/services/d1/backend.rs:
##########
@@ -0,0 +1,263 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::fmt::Formatter;
+
+use async_trait::async_trait;
+use http::header;
+use http::Request;
+use http::StatusCode;
+
+use crate::raw::adapters::kv;
+use crate::raw::*;
+use crate::*;
+
+use super::error::parse_error;
+
+#[doc = include_str!("docs.md")]
+#[derive(Default)]
+pub struct D1Builder {
+    root: Option<String>,
+    endpoint: Option<String>,
+    sql: Option<String>,

Review Comment:
   SQL should be constructed by OpenDAL itself instead of in the config.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(service/d1): Support d1 for opendal [incubator-opendal]

Posted by "realtaobo (via GitHub)" <gi...@apache.org>.
realtaobo commented on PR #3248:
URL: https://github.com/apache/incubator-opendal/pull/3248#issuecomment-1759266022

   @Xuanwo I passed the local behavior, but d1 limited the [Maximum string, BLOB or table row size](https://developers.cloudflare.com/d1/platform/limits/#:~:text=1%2C000%2C000%20bytes%20(1%20MB)) to `1M`, so before that I reduced [gen_bytes](https://github.com/realtaobo/incubator-opendal/blob/efc9879a29ef3d0d804dbde02740491b9e4706e4/core/tests/behavior/utils.rs#L110)  locally.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org