Merge pull request #3390 from albinsuresh/imp/entity-delete-rest-api-…

…updates

feat: Return entity metadata in delete REST API response

crates/common/mqtt_channel/src/topics.rs

@@ -6,13 +6,21 @@ use serde::Deserialize;
 use serde::Serialize;
 use std::collections::HashSet;
 use std::convert::TryInto;
+use std::fmt::Display;
+use std::fmt::Formatter;
 
 /// An MQTT topic
 #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
 pub struct Topic {
     pub name: String,
 }
 
+impl Display for Topic {
+    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
+        Display::fmt(&self.name, f)
+    }
+}
+
 impl Topic {
     /// Check if the topic name is valid and build a new topic.
     pub fn new(name: &str) -> Result<Topic, MqttError> {

crates/core/tedge_agent/src/entity_manager/server.rs

@@ -30,7 +30,7 @@ pub enum EntityStoreRequest {
 pub enum EntityStoreResponse {
     Get(Option<EntityMetadata>),
     Create(Result<Vec<RegisteredEntityData>, entity_store::Error>),
-    Delete(Vec<EntityTopicId>),
+    Delete(Vec<EntityMetadata>),
     List(Vec<EntityMetadata>),
     Ok,
 }
@@ -194,18 +194,18 @@ impl EntityStoreServer {
         Ok(registered)
     }
 
-    async fn deregister_entity(&mut self, topic_id: EntityTopicId) -> Vec<EntityTopicId> {
+    async fn deregister_entity(&mut self, topic_id: EntityTopicId) -> Vec<EntityMetadata> {
         let deleted = self.entity_store.deregister_entity(&topic_id);
-        for topic_id in deleted.iter() {
+        for entity in deleted.iter() {
             let topic = self
                 .mqtt_schema
-                .topic_for(topic_id, &Channel::EntityMetadata);
+                .topic_for(&entity.topic_id, &Channel::EntityMetadata);
             let clear_entity_msg = MqttMessage::new(&topic, "")
                 .with_retain()
                 .with_qos(QoS::AtLeastOnce);
 
             if let Err(err) = self.mqtt_publisher.send(clear_entity_msg).await {
-                error!("Failed to publish clear message for the topic: {topic_id} due to {err}",)
+                error!("Failed to publish clear message for the topic: {topic} due to {err}",)
             }
         }
 

crates/core/tedge_agent/src/entity_manager/tests.rs

@@ -80,7 +80,10 @@ async fn check_registrations(registrations: Commands) {
                 .iter()
                 .map(|registered_entity| registered_entity.reg_message.topic_id.clone())
                 .collect(),
-            EntityStoreResponse::Delete(actual_updates) => HashSet::from_iter(actual_updates),
+            EntityStoreResponse::Delete(actual_updates) => actual_updates
+                .into_iter()
+                .map(|meta| meta.topic_id)
+                .collect(),
             _ => HashSet::new(),
         };
         assert_eq!(actual_updates, expected_updates);

crates/core/tedge_agent/src/http_server/entity_store.rs

@@ -186,7 +186,7 @@ async fn get_entity(
 async fn deregister_entity(
     State(state): State<AgentState>,
     Path(path): Path<String>,
-) -> Result<Json<Vec<EntityTopicId>>, Error> {
+) -> Result<Response, Error> {
     let topic_id = EntityTopicId::from_str(&path)?;
 
     let response = state
@@ -199,7 +199,11 @@ async fn deregister_entity(
         return Err(Error::InvalidEntityStoreResponse);
     };
 
-    Ok(Json(deleted))
+    if deleted.is_empty() {
+        return Ok(StatusCode::NO_CONTENT.into_response());
+    }
+
+    Ok((StatusCode::OK, Json(deleted)).into_response())
 }
 
 async fn list_entities(
@@ -490,11 +494,11 @@ mod tests {
         tokio::spawn(async move {
             if let Some(mut req) = entity_store_box.recv().await {
                 if let EntityStoreRequest::Delete(topic_id) = req.request {
-                    let target_topic_id =
-                        EntityTopicId::default_child_device("test-child").unwrap();
-                    if topic_id == target_topic_id {
+                    let target_entity =
+                        EntityMetadata::child_device("test-child".to_string()).unwrap();
+                    if topic_id == target_entity.topic_id {
                         req.reply_to
-                            .send(EntityStoreResponse::Delete(vec![target_topic_id]))
+                            .send(EntityStoreResponse::Delete(vec![target_entity]))
                             .await
                             .unwrap();
                     }
@@ -513,15 +517,15 @@ mod tests {
 
         assert_eq!(response.status(), StatusCode::OK);
         let body = response.into_body().collect().await.unwrap().to_bytes();
-        let deleted: Vec<EntityTopicId> = serde_json::from_slice(&body).unwrap();
+        let deleted: Vec<EntityMetadata> = serde_json::from_slice(&body).unwrap();
         assert_eq!(
             deleted,
-            vec![EntityTopicId::default_child_device("test-child").unwrap()]
+            vec![EntityMetadata::child_device("test-child".to_string()).unwrap()]
         );
     }
 
     #[tokio::test]
-    async fn delete_unknown_entity_is_ok() {
+    async fn delete_unknown_entity_returns_no_content() {
         let TestHandle {
             mut app,
             mut entity_store_box,
@@ -549,7 +553,7 @@ mod tests {
             .expect("request builder");
 
         let response = app.call(req).await.unwrap();
-        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(response.status(), StatusCode::NO_CONTENT);
     }
 
     #[tokio::test]

crates/core/tedge_api/src/entity_store.rs

@@ -442,7 +442,7 @@ impl EntityStore {
     }
 
     /// Recursively deregister an entity, its child devices and services
-    pub fn deregister_entity(&mut self, topic_id: &EntityTopicId) -> Vec<EntityTopicId> {
+    pub fn deregister_entity(&mut self, topic_id: &EntityTopicId) -> Vec<EntityMetadata> {
         let mut removed_entities = vec![];
         self.entities.remove(topic_id, &mut removed_entities);
         removed_entities
@@ -454,7 +454,7 @@ impl EntityStore {
     pub fn deregister_and_persist_entity(
         &mut self,
         topic_id: &EntityTopicId,
-    ) -> Result<Vec<EntityTopicId>, Error> {
+    ) -> Result<Vec<EntityMetadata>, Error> {
         let removed_entities = self.deregister_entity(topic_id);
 
         if !removed_entities.is_empty() {
@@ -690,19 +690,14 @@ impl EntityTree {
 
     /// Recursively remove an entity, its child devices and services
     ///
-    /// Populate the given vector with the topic identifiers of the removed entities
-    fn remove(&mut self, topic_id: &EntityTopicId, removed_entities: &mut Vec<EntityTopicId>) {
-        if let Some(children) = self
-            .entities
-            .get(topic_id)
-            .map(|node| node.children.clone())
-        {
+    /// Populate the given vector with the metadata of the removed entities
+    fn remove(&mut self, topic_id: &EntityTopicId, removed_entities: &mut Vec<EntityMetadata>) {
+        if let Some(node) = self.entities.remove(topic_id) {
+            removed_entities.push(node.metadata);
+            let children = node.children;
             children
                 .iter()
                 .for_each(|sub_topic| self.remove(sub_topic, removed_entities));
-
-            self.entities.remove(topic_id);
-            removed_entities.push(topic_id.clone())
         }
     }
 
@@ -1887,7 +1882,11 @@ mod tests {
         register_child(&mut store, "device/003//", "device/00D//");
         register_child(&mut store, "device/003//", "device/00E//");
 
-        let mut removed = store.deregister_entity(&entity("device/002//"));
+        let mut removed = store
+            .deregister_entity(&entity("device/002//"))
+            .into_iter()
+            .map(|v| v.topic_id)
+            .collect::<Vec<_>>();
         removed.sort_by(|a, b| a.as_str().cmp(b.as_str()));
 
         assert_eq!(

docs/src/operate/registration/register.md

@@ -442,15 +442,32 @@ The deleted entities are cleared from the MQTT broker as well.
 DELETE /v1/entities/{topic-id}
 ```
 
-**Responses**
-
-* 200: OK
-  ```json
-  ["device/child01//"]
-  ```
-
 **Example**
 
 ```shell
-curl -X DELETE http://localhost:8000/tedge/entity-store/v1/entities/device/child01
+curl -X DELETE http://localhost:8000/tedge/entity-store/v1/entities/device/child21
 ```
+
+**Responses**
+
+* 200: OK, when entities are deleted
+  ```json
+  [
+    {
+        "@topic-id": "device/child21//",
+        "@type": "child-device",
+        "@parent": "device/child2//"
+    },
+    {
+        "@topic-id": "device/child21/service/service210",
+        "@type": "service",
+        "@parent": "device/child21//"
+    },
+    {
+        "@topic-id": "device/child210//",
+        "@type": "child-device",
+        "@parent": "device/child21//"
+    }
+  ]
+  ```
+* 204: No Content, when nothing is deleted

tests/RobotFramework/libraries/ThinEdgeIO/ThinEdgeIO.py

@@ -1013,6 +1013,43 @@ def register_entity(
         json_output = json.loads(output.stdout)
         return json_output
 
+    @keyword("Deregister Entity")
+    def deregister_entity(
+            self,
+            topic_id: str,
+            device_name: str = None 
+    ) -> Dict[str, Any]:
+        """
+        Delete the given entity and its child tree from the entity store
+
+        Args:
+            topic_id (str, optional): Topic ID of the entity to be deleted
+            device_name (str, optional): Device name to perform this action from
+
+        Returns:
+            Dict[str, Any]: Registered entity topic ID
+
+        *Example:*
+        | ${entities}= | Delete Entity | device/child0// |
+        | ${entities}= | Delete Entity | device/child0/service/service0 | device_name=${PARENT_SN} |
+        """
+        device = self.current
+        if device_name:
+            if device_name in self.devices:
+                device = self.devices.get(device_name)
+
+        if not device:
+            raise ValueError(
+                f"Unable to query the entity store as the device: '{device_name}' has not been setup"
+            )
+
+        command = (
+            f"curl -X DELETE http://localhost:8000/tedge/entity-store/v1/entities/{topic_id}"
+        )
+        output = device.execute_command(command)
+        json_output = json.loads(output.stdout)
+        return json_output
+
     @keyword("List Entities")
     def list_entities(
             self,
@@ -1117,6 +1154,47 @@ def assert_contains_entity(
 
         return matches
 
+    @keyword("Should Not Contain Entity")
+    def assert_does_not_contain_entity(
+            self,
+            topic_id: str,
+            entities: List[Dict[str, Any]] = None,
+            device_name: str = None,
+            **kwargs
+    ) -> List[Dict[str, Any]]:
+        """Assert that the entity store does not contains the given entity
+
+        Args:
+            topic_id (str, optional): Topic ID of the entity
+            entities (List[Dict[str, Any]], optional): List of entities to search in. Defaults to None.
+            device_name (str, optional): Device name to fetch the entity list from
+
+        Returns:
+            List[Dict[str, Any]]: List of entities matching the given entity definition
+
+        *Example:*
+        | ${entities}= | Should Not Contain Entity | topic_id=device/child123// |
+        | ${entities}= | Should Not Contain Entity | topic_id=device/child123// | entities=${entity_list_json} |
+        | ${entities}= | Should Not Contain Entity | topic_id=device/child123// | entities=${entity_list_json} | device_name=${PARENT_SN} |
+        """
+        device = self.current
+        if device_name:
+            if device_name in self.devices:
+                device = self.devices.get(device_name)
+
+        if not device:
+            raise ValueError(
+                f"Unable to query the entity store as the device: '{device_name}' has not been setup"
+            )
+
+        if not entities:
+            entities = self.list_entities()
+
+        assert all(entity["@topic-id"] != topic_id for entity in entities)
+
+        return entities
+
+
 def to_date(value: relativetime_) -> datetime:
     if isinstance(value, datetime):
         return value

tests/RobotFramework/tests/tedge_agent/http_server/entity_store_api.robot

@@ -28,9 +28,15 @@ CRUD apis
     ${entities}=    Execute Command    curl http://localhost:8000/tedge/entity-store/v1/entities
     Should Contain    ${entities}    {"@topic-id":"device/child01//","@parent":"device/main//","@type":"child-device"}
 
-    ${status}=    Execute Command
-    ...    curl -o /dev/null --silent --write-out "%\{http_code\}" -X DELETE http://localhost:8000/tedge/entity-store/v1/entities/device/child01//
-    Should Be Equal    ${status}    200
+    ${timestamp}=    Get Unix Timestamp
+    ${delete}=    Execute Command
+    ...    curl --silent -X DELETE http://localhost:8000/tedge/entity-store/v1/entities/device/child01//
+    Should Be Equal
+    ...    ${delete}
+    ...    [{"@topic-id":"device/child01//","@parent":"device/main//","@type":"child-device"}]
+    Should Have MQTT Messages
+    ...    te/device/child01//
+    ...    date_from=${timestamp}
 
     ${get}=    Execute Command
     ...    curl -o /dev/null --silent --write-out "%\{http_code\}" http://localhost:8000/tedge/entity-store/v1/entities/device/child01//
@@ -56,6 +62,50 @@ Entity auto-registration over MQTT
     ...    te/device/auto_child/service/collectd
     ...    message_contains={"@parent":"device/auto_child//","@type":"service","name":"collectd","type":"service"}
 
+Delete entity tree
+    Register Entity    device/child0//    child-device    device/main//
+    Register Entity    device/child1//    child-device    device/main//
+    Register Entity    device/child2//    child-device    device/main//
+    Register Entity    device/child0/service/service0    service    device/child0//
+    Register Entity    device/child00//    child-device    device/child0//
+    Register Entity    device/child000//    child-device    device/child00//
+
+    ${deleted}=    Deregister Entity    device/child0//
+    Length Should Be    ${deleted}    4
+
+    # Assert the deleted entities
+    Should Contain Entity
+    ...    {"@topic-id":"device/child0//","@parent":"device/main//","@type":"child-device"}
+    ...    ${deleted}
+    Should Contain Entity
+    ...    {"@topic-id":"device/child00//","@parent":"device/child0//","@type":"child-device"}
+    ...    ${deleted}
+    Should Contain Entity
+    ...    {"@topic-id":"device/child0/service/service0","@parent":"device/child0//","@type":"service","type":"service"}
+    ...    ${deleted}
+    Should Contain Entity
+    ...    {"@topic-id":"device/child000//","@parent":"device/child00//","@type":"child-device"}
+    ...    ${deleted}
+
+    ${entities}=    List Entities
+    Should Not Contain Entity
+    ...    "device/child0//"
+    ...    ${entities}
+    Should Not Contain Entity
+    ...    "device/child00//"
+    ...    ${entities}
+    Should Not Contain Entity
+    ...    "device/child000//"
+    ...    ${entities}
+
+    # Assert the remaining entities
+    Should Contain Entity
+    ...    {"@topic-id":"device/child1//","@parent":"device/main//","@type":"child-device"}
+    ...    ${entities}
+    Should Contain Entity
+    ...    {"@topic-id":"device/child2//","@parent":"device/main//","@type":"child-device"}
+    ...    ${entities}
+
 
 *** Keywords ***
 Custom Setup