Improve the name mechanism of vineyard objects.

* Make sure one name is associated with one vineyard object.
* Fix the name can't be all digits.
* Support delete vineyard object by name.

Signed-off-by: Ye Cao <[email protected]>

python/client.cc

@@ -487,6 +487,18 @@ void bind_client(py::module& mod) {
           },
           "object_id"_a, py::arg("wait") = false,
           py::call_guard<py::gil_scoped_release>())
+      .def(
+          "name_exists",
+          [](ClientBase* self, std::string const& name) -> bool {
+            ObjectID object_id;
+            auto status = self->GetName(name, object_id, false);
+            if (status.ok()) {
+              return true;
+            } else {
+              return false;
+            }
+          },
+          "name"_a)
       .def(
           "drop_name",
           [](ClientBase* self, std::string const& name) {

python/pybind11_docs.cc

@@ -743,6 +743,20 @@ Get the associated object id of the given name.
     ObjectID: The associated object id with the name.
 )doc";
 
+const char* ClientBase_name_exists = R"doc(
+.. method:: name_exists(name: str or ObjectName) -> bool
+    :noindex:
+
+Check if the given name exists in the vineyard server.
+
+Parameters:
+    name: str
+        The name that will be checked.
+
+Returns:
+    bool: :code:`True` if the name exists in the vineyard server.
+)doc";
+
 const char* ClientBase_list_names = R"doc(
 .. method:: list_names(pattern: str, regex: bool = False, limit: int = 5)
                        -> Dict[str, ObjectID]

python/vineyard/core/builder.py

@@ -156,7 +156,7 @@ def put(
     builder: Optional[BuilderContext] = None,
     persist: bool = False,
     name: Optional[str] = None,
-    **kwargs
+    **kwargs,
 ):
     """Put python value to vineyard.
 
@@ -191,6 +191,9 @@ def put(
     Returns:
         ObjectID: The result object id will be returned.
     """
+    if name is not None and client.name_exists(name):
+        raise ValueError(f"Name {name} already exists in the vineyard")
+
     if builder is not None:
         return builder(client, value, **kwargs)
 

python/vineyard/core/client.py

@@ -370,12 +370,18 @@ def create_metadata(
     @_apply_docstring(IPCClient.delete)
     def delete(
         self,
-        object: Union[ObjectID, Object, ObjectMeta, List[ObjectID]],
+        object_id: Union[ObjectID, Object, ObjectMeta, List[ObjectID]] = None,
+        name: str = None,
         force: bool = False,
         deep: bool = True,
         memory_trim: bool = False,
     ) -> None:
-        return self.default_client().delete(object, force, deep, memory_trim)
+        if object_id is None and name is None:
+            raise ValueError("Either object_id or name should be provided.")
+        if name is not None:
+            object_id = self.default_client().get_name(name)
+            self.default_client().drop_name(name)
+        return self.default_client().delete(object_id, force, deep, memory_trim)
 
     @_apply_docstring(IPCClient.create_stream)
     def create_stream(self, id: ObjectID) -> None:
@@ -439,6 +445,10 @@ def put_name(self, object: Union[Object, ObjectMeta, ObjectID], name: str) -> No
     def get_name(self, name: str, wait: bool = False) -> ObjectID:
         return self.default_client().get_name(name, wait)
 
+    @_apply_docstring(IPCClient.name_exists)
+    def name_exists(self, name: str) -> bool:
+        return self.default_client().name_exists(name)
+
     @_apply_docstring(IPCClient.drop_name)
     def drop_name(self, name: str) -> None:
         return self.default_client().drop_name(name)

src/server/server/vineyard_server.cc

@@ -57,6 +57,14 @@ namespace vineyard {
   } while (0)
 #endif  // ENSURE_VINEYARDD_READY
 
+// Helper function to adjust the name if it's all digits.
+std::string AdjustName(const std::string& name) {
+  if (std::regex_match(name, std::regex("[0-9]+$"))) {
+    return "_" + name;
+  }
+  return name;
+}
+
 bool DeferredReq::Alive() const { return alive_fn_(); }
 
 bool DeferredReq::TestThenCall(const json& meta) const {
@@ -871,13 +879,22 @@ Status VineyardServer::PutName(const ObjectID object_id,
                                const std::string& name, callback_t<> callback) {
   ENSURE_VINEYARDD_READY();
   auto self(shared_from_this());
+  // if the name is all digits, prefix it with '_'
+  // as the nlohmann/json can't convert '/names/1234: 12345' to 'names:{"1234",
+  // "12345"}'
+  std::string new_name = AdjustName(name);
+  std::cout << "new_name is: " << new_name << std::endl;
   meta_service_ptr_->RequestToPersist(
-      [object_id, name](const Status& status, const json& meta,
-                        std::vector<meta_tree::op_t>& ops) {
+      [object_id, name = new_name](const Status& status, const json& meta,
+                                   std::vector<meta_tree::op_t>& ops) {
         if (status.ok()) {
           // TODO: do proper validation:
           // 1. global objects can have name, local ones cannot.
-          // 2. the name-object_id mapping shouldn't be overwrite.
+
+          auto names = meta.value("names", json(nullptr));
+          if (names.is_object() && names.contains(name)) {
+            return Status::ObjectExists("name " + name + " already exists");
+          }
 
           // blob cannot have name
           if (IsBlob(object_id)) {
@@ -928,9 +945,11 @@ Status VineyardServer::GetName(const std::string& name, const bool wait,
                                callback_t<const ObjectID&> callback) {
   ENSURE_VINEYARDD_READY();
   auto self(shared_from_this());
-  meta_service_ptr_->RequestToGetData(true, [self, name, wait, alive, callback](
-                                                const Status& status,
-                                                const json& meta) {
+  std::string new_name = AdjustName(name);
+
+  meta_service_ptr_->RequestToGetData(true, [self, name = new_name, wait, alive,
+                                             callback](const Status& status,
+                                                       const json& meta) {
     if (status.ok()) {
       auto test_task = [name](const json& meta) -> bool {
         auto names = meta.value("names", json(nullptr));
@@ -967,10 +986,12 @@ Status VineyardServer::GetName(const std::string& name, const bool wait,
 Status VineyardServer::DropName(const std::string& name,
                                 callback_t<> callback) {
   ENSURE_VINEYARDD_READY();
+  std::string new_name = AdjustName(name);
+
   auto self(shared_from_this());
   meta_service_ptr_->RequestToPersist(
-      [name](const Status& status, const json& meta,
-             std::vector<meta_tree::op_t>& ops) {
+      [name = new_name](const Status& status, const json& meta,
+                        std::vector<meta_tree::op_t>& ops) {
         if (status.ok()) {
           auto names = meta.value("names", json(nullptr));
           if (names.is_object()) {