Xuanwo commented on code in PR #2782:
URL: 
https://github.com/apache/incubator-opendal/pull/2782#discussion_r1285671838


##########
bindings/php/opendal-php.stubs.php:
##########
@@ -20,6 +20,111 @@
 
 // Stubs for opendal-php
 
-namespace {
-    function debug(): string {}
+namespace OpenDAL {
+    class Operator {
+        public function __construct(string $scheme_str, array $config) {}
+
+        /**
+         * Write bytes into given path.
+         */
+        public function write(string $path, string $content): mixed {}
+
+        /**
+         * Read the whole path into bytes.
+         */
+        public function read(string $path): string {}

Review Comment:
   The returning content could also be bytes.



##########
bindings/php/opendal-php.stubs.php:
##########
@@ -20,6 +20,111 @@
 
 // Stubs for opendal-php
 
-namespace {
-    function debug(): string {}
+namespace OpenDAL {
+    class Operator {
+        public function __construct(string $scheme_str, array $config) {}
+
+        /**
+         * Write bytes into given path.
+         */
+        public function write(string $path, string $content): mixed {}
+
+        /**
+         * Read the whole path into bytes.
+         */
+        public function read(string $path): string {}
+
+        /**
+         * Check if this path exists or not.
+         */
+        public function is_exist(string $path): int {}

Review Comment:
   Maybe we should add docs about it's meaning: `0` for `false` and `1` for 
`true`?



##########
bindings/php/src/lib.rs:
##########
@@ -15,14 +15,172 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use ::opendal as od;
+use ext_php_rs::binary::Binary;
+use ext_php_rs::convert::FromZval;
+use ext_php_rs::exception::PhpException;
+use ext_php_rs::flags::DataType;
 use ext_php_rs::prelude::*;
-use opendal::{EntryMode, Metadata};
+use ext_php_rs::types::Zval;
+use std::collections::HashMap;
+use std::str::FromStr;
 
-#[php_function]
-pub fn debug() -> String {
-    let metadata = Metadata::new(EntryMode::FILE);
+#[php_class(name = "OpenDAL\\Operator")]
+pub struct Operator(od::BlockingOperator);
 
-    format!("{:?}", metadata)
+#[php_impl(rename_methods = "none")]
+impl Operator {
+    pub fn __construct(scheme_str: String, config: HashMap<String, String>) -> 
PhpResult<Self> {
+        let scheme = 
od::Scheme::from_str(&scheme_str).map_err(format_php_err)?;
+        let op = od::Operator::via_map(scheme, 
config).map_err(format_php_err)?;
+
+        Ok(Operator(op.blocking()))
+    }
+
+    /// Write string into given path.
+    pub fn write(&self, path: &str, content: String) -> PhpResult<()> {
+        self.0.write(path, content).map_err(format_php_err)
+    }
+
+    /// Write bytes into given path, binary safe.
+    pub fn write_binary(&self, path: &str, content: Vec<u8>) -> PhpResult<()> {
+        self.0.write(path, content).map_err(format_php_err)
+    }
+
+    /// Read the whole path into bytes, binary safe.
+    pub fn read_binary(&self, path: &str) -> PhpResult<Binary<u8>> {
+        self.0.read(path).map_err(format_php_err).map(Binary::from)
+    }
+
+    /// Read the whole path into bytes.
+    pub fn read(&self, path: &str) -> PhpResult<String> {
+        self.0
+            .read(path)
+            .map(|res| String::from_utf8(res).unwrap())
+            .map_err(format_php_err)
+    }
+
+    /// Check if this path exists or not.
+    pub fn is_exist(&self, path: &str) -> PhpResult<u8> {
+        self.0
+            .is_exist(path)
+            .map_err(format_php_err)
+            .map(|b| if b { 1 } else { 0 })
+    }
+
+    /// Get current path's metadata **without cache** directly.
+    ///
+    /// # Notes
+    ///
+    /// Use `stat` if you:
+    ///
+    /// - Want detect the outside changes of path.
+    /// - Don't want to read from cached metadata.
+    pub fn stat(&self, path: &str) -> PhpResult<Metadata> {
+        self.0.stat(path).map_err(format_php_err).map(Metadata)
+    }
+
+    /// Delete given path.
+    ///
+    /// # Notes
+    ///
+    /// - Delete not existing error won't return errors.
+    pub fn delete(&self, path: &str) -> PhpResult<()> {
+        self.0.delete(path).map_err(format_php_err)
+    }
+
+    /// Create a dir at given path.
+    ///
+    /// # Notes
+    ///
+    /// To indicate that a path is a directory, it is compulsory to include
+    /// a trailing / in the path. Failure to do so may result in
+    /// `NotADirectory` error being returned by OpenDAL.
+    ///
+    /// # Behavior
+    ///
+    /// - Create on existing dir will succeed.
+    /// - Create dir is always recursive, works like `mkdir -p`
+    pub fn create_dir(&self, path: &str) -> PhpResult<()> {
+        self.0.create_dir(path).map_err(format_php_err)
+    }
+}
+
+#[php_class(name = "OpenDAL\\Metadata")]
+pub struct Metadata(od::Metadata);
+
+#[php_impl(rename_methods = "none")]
+impl Metadata {
+    #[getter]
+    pub fn content_disposition(&self) -> Option<String> {
+        self.0.content_disposition().map(|s| s.to_string())
+    }
+
+    /// Content length of this entry.
+    #[getter]
+    pub fn content_length(&self) -> u64 {
+        self.0.content_length()
+    }
+
+    /// Content MD5 of this entry.
+    #[getter]
+    pub fn content_md5(&self) -> Option<String> {
+        self.0.content_md5().map(|s| s.to_string())
+    }
+
+    /// Content Type of this entry.
+    #[getter]
+    pub fn content_type(&self) -> Option<String> {
+        self.0.content_type().map(|s| s.to_string())
+    }
+
+    /// ETag of this entry.
+    #[getter]
+    pub fn etag(&self) -> Option<String> {
+        self.0.etag().map(|s| s.to_string())
+    }
+
+    /// mode represent this entry's mode.
+    #[getter]
+    pub fn mode(&self) -> EntryMode {
+        EntryMode(self.0.mode())
+    }
+}
+
+#[php_class(name = "OpenDAL\\EntryMode")]
+pub struct EntryMode(od::EntryMode);
+
+impl<'b> FromZval<'b> for EntryMode {
+    const TYPE: DataType = DataType::Object(Some("OpenDAL\\EntryMode"));
+
+    fn from_zval(zval: &'b Zval) -> Option<Self> {
+        zval.object().and_then(|obj| obj.get_property("mode").ok())
+    }
+}
+
+#[php_impl(rename_methods = "none")]
+impl EntryMode {
+    #[getter]
+    pub fn is_dir(&self) -> u8 {
+        match self.0.is_dir() {
+            true => 1,
+            false => 0,
+        }
+    }
+
+    #[getter]
+    pub fn is_file(&self) -> u8 {
+        match self.0.is_file() {
+            true => 1,
+            false => 0,
+        }
+    }
+}
+
+fn format_php_err(e: od::Error) -> PhpException {
+    // @todo use custom exception, we cannot use custom exception now,

Review Comment:
   We can record this in a tracking issues of php binding.



##########
bindings/php/opendal-php.stubs.php:
##########
@@ -20,6 +20,111 @@
 
 // Stubs for opendal-php
 
-namespace {
-    function debug(): string {}
+namespace OpenDAL {
+    class Operator {
+        public function __construct(string $scheme_str, array $config) {}
+
+        /**
+         * Write bytes into given path.
+         */
+        public function write(string $path, string $content): mixed {}
+
+        /**
+         * Read the whole path into bytes.
+         */
+        public function read(string $path): string {}
+
+        /**
+         * Check if this path exists or not.
+         */
+        public function is_exist(string $path): int {}
+
+        /**
+         * Get current path's metadata **without cache** directly.
+         *
+         * # Notes
+         *
+         * Use `stat` if you:
+         *
+         * - Want detect the outside changes of path.
+         * - Don't want to read from cached metadata.
+         */
+        public function stat(string $path): \OpenDAL\Metadata {}
+
+        /**
+         * Delete given path.
+         *
+         * # Notes
+         *
+         * - Delete not existing error won't return errors.
+         */
+        public function delete(string $path): mixed {}

Review Comment:
   What's this `mixed` for?



##########
bindings/php/tests/Feature/BasicIOTest.php:
##########
@@ -0,0 +1,74 @@
+<?php
+/*
+ * 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.
+ */
+
+describe('basic io', function () {
+    $op = new \OpenDAL\Operator('fs', ['root' => '/tmp']);

Review Comment:
   we can test against with memory service.



##########
bindings/php/opendal-php.stubs.php:
##########
@@ -20,6 +20,111 @@
 
 // Stubs for opendal-php
 
-namespace {
-    function debug(): string {}
+namespace OpenDAL {
+    class Operator {
+        public function __construct(string $scheme_str, array $config) {}
+
+        /**
+         * Write bytes into given path.
+         */
+        public function write(string $path, string $content): mixed {}
+
+        /**
+         * Read the whole path into bytes.
+         */
+        public function read(string $path): string {}
+
+        /**
+         * Check if this path exists or not.
+         */
+        public function is_exist(string $path): int {}
+
+        /**
+         * Get current path's metadata **without cache** directly.
+         *
+         * # Notes
+         *
+         * Use `stat` if you:
+         *
+         * - Want detect the outside changes of path.
+         * - Don't want to read from cached metadata.
+         */
+        public function stat(string $path): \OpenDAL\Metadata {}
+
+        /**
+         * Delete given path.
+         *
+         * # Notes
+         *
+         * - Delete not existing error won't return errors.
+         */
+        public function delete(string $path): mixed {}
+
+        /**
+         * Create a dir at given path.
+         *
+         * # Notes
+         *
+         * To indicate that a path is a directory, it is compulsory to include
+         * a trailing / in the path. Failure to do so may result in
+         * `NotADirectory` error being returned by OpenDAL.
+         *
+         * # Behavior
+         *
+         * - Create on existing dir will succeed.
+         * - Create dir is always recursive, works like `mkdir -p`
+         */
+        public function create_dir(string $path): mixed {}
+    }
+
+    class Metadata {
+        public $mode;
+
+        public $content_type;
+
+        public $etag;
+
+        public $content_md5;
+
+        public $content_disposition;
+
+        public $content_length;
+
+        public function content_disposition(): ?string {}
+
+        /**
+         * Content length of this entry.
+         */
+        public function content_length(): int {}

Review Comment:
   What's the range of php's int? Will it overflow if we have int64::MAX?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to