opendal icon indicating copy to clipboard operation
opendal copied to clipboard

refactor(core): move move logic from fn metadata(&self) -> Arc<AccessorInfo> to impl<A: Access> Layer<A> for CompleteLayer

Open Lzzzzzt opened this issue 1 year ago • 1 comments

Which issue does this PR close?

Closes #4888

What changes are included in this PR?

mentioned in #4888

Are there any user-facing changes?

No

Lzzzzzt avatar Jul 14 '24 03:07 Lzzzzzt

The C/C++ test failed, which is a bit weird. I'll try to figure out what happened. I'm guessing it's related to our Arc been dropped while C still hold it.

Xuanwo avatar Jul 16 '24 10:07 Xuanwo

I'm curious about the C binding error in CI, so I debugged it locally. The specific reason should be that the changes in this PR have altered the way the CompleteLayer::complete_blocking_create_dir is called, resulting in the opendal_operator_create_dir method in the C binding returning an error.

bindings/c/src/tests/bdd.cpp:

error = opendal_operator_create_dir(this->p, "tmpdir/");  // <====== the error is not nullptr
EXPECT_EQ(error, nullptr);

bindings/c/src/operator.rs

#[no_mangle]
pub unsafe extern "C" fn opendal_operator_create_dir(
    op: &opendal_operator,
    path: *const c_char,
) -> *mut opendal_error {
    assert!(!path.is_null());
    let path = std::ffi::CStr::from_ptr(path)
        .to_str()
        .expect("malformed path");
    if let Err(err) = op.deref().create_dir(path) {
        println!("create dir error: {err}");  // <========
        opendal_error::new(err)
    } else {
        std::ptr::null_mut()
    }
}

core/src/layers/complete.rs

fn complete_blocking_create_dir(&self, path: &str, args: OpCreateDir) -> Result<RpCreateDir> {
    let capability = self.info.full_capability();
    println!(
        "complete_blocking_create_dir: full capability = {:?}",
        capability
    );
    if capability.create_dir && capability.blocking {
        println!("complete_blocking_create_dir: create_dir && blocking");   <===== after this PR
        return self.inner().blocking_create_dir(path, args);
    }
    if capability.write_can_empty && capability.list && capability.blocking {
        println!("complete_blocking_create_dir: write_can_empty && list && blocking");   <====== before this PR
        let (_, mut w) = self.inner.blocking_write(path, OpWrite::default())?;
        oio::BlockingWrite::close(&mut w)?;
        return Ok(RpCreateDir::default());
    }

    Err(self.new_unsupported_error(Operation::BlockingCreateDir))
}

error:

The memory service does not support the blocking_create_dir operation.

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OpendalBddTest
[ RUN      ] OpendalBddTest.FeatureTest
complete_blocking_create_dir: full capability = { Stat | Read | Write | CreateDir | Delete | List | Blocking }
complete_blocking_create_dir: create_dir && blocking
create dir error: Unsupported (permanent) at blocking_create_dir, context: { service: memory, path: tmpdir/ } => operation is not supported
/Users/qinxuan/Code/apache/opendal/bindings/c/tests/bdd.cpp:127: Failure
Expected equality of these values:
  error
    Which is: 0x603000004c30
  nullptr
    Which is: (nullptr)

/Users/qinxuan/Code/apache/opendal/bindings/c/tests/bdd.cpp:129: Failure
Expected equality of these values:
  stat.error
    Which is: 0x603000004e10
  nullptr
    Which is: (nullptr)

koushiro avatar Nov 05 '24 05:11 koushiro

Should be fixed by https://github.com/apache/opendal/pull/5662

Xuanwo avatar Feb 26 '25 05:02 Xuanwo