[DPE-2876] Fix on fetching local databag (when relation is broken) (#109

)

* [FIX] Local databag is accessible on relation broken

* [FIX] Safeguarding pop()

* Tests for safe delete function

lib/charms/data_platform_libs/v0/data_interfaces.py

@@ -320,7 +320,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 22
+LIBPATCH = 23
 
 PYDEPS = ["ops>=2.0.0"]
 
@@ -807,6 +807,9 @@ def _fetch_relation_data_without_secrets(
         This is used typically when the Provides side wants to read the Requires side's data,
         or when the Requires side may want to read its own data.
         """
+        if app not in relation.data or not relation.data[app]:
+            return {}
+
         if fields:
             return {k: relation.data[app][k] for k in fields if k in relation.data[app]}
         else:
@@ -830,6 +833,9 @@ def _fetch_relation_data_with_secrets(
         normal_fields = []
 
         if not fields:
+            if app not in relation.data or not relation.data[app]:
+                return {}
+
             all_fields = list(relation.data[app].keys())
             normal_fields = [field for field in all_fields if not self._is_secret_field(field)]
 
@@ -853,8 +859,11 @@ def _fetch_relation_data_with_secrets(
 
     def _update_relation_data_without_secrets(
         self, app: Application, relation: Relation, data: Dict[str, str]
-    ):
+    ) -> None:
         """Updating databag contents when no secrets are involved."""
+        if app not in relation.data or relation.data[app] is None:
+            return
+
         if any(self._is_secret_field(key) for key in data.keys()):
             raise SecretsIllegalUpdateError("Can't update secret {key}.")
 
@@ -865,8 +874,19 @@ def _delete_relation_data_without_secrets(
         self, app: Application, relation: Relation, fields: List[str]
     ) -> None:
         """Remove databag fields 'fields' from Relation."""
+        if app not in relation.data or not relation.data[app]:
+            return
+
         for field in fields:
-            relation.data[app].pop(field)
+            try:
+                relation.data[app].pop(field)
+            except KeyError:
+                logger.debug(
+                    "Non-existing field was attempted to be removed from the databag %s, %s",
+                    str(relation.id),
+                    str(field),
+                )
+                pass
 
     # Public interface methods
     # Handling Relation Fields seamlessly, regardless if in databag or a Juju Secret
@@ -880,9 +900,6 @@ def get_relation(self, relation_name, relation_id) -> Relation:
                 "Relation %s %s couldn't be retrieved", relation_name, relation_id
             )
 
-        if not relation.app:
-            raise DataInterfacesError("Relation's application missing")
-
         return relation
 
     def fetch_relation_data(
@@ -1089,7 +1106,10 @@ def _delete_relation_secret(
         # Remove secret from the relation if it's fully gone
         if not new_content:
             field = self._generate_secret_field_name(group)
-            relation.data[self.local_app].pop(field)
+            try:
+                relation.data[self.local_app].pop(field)
+            except KeyError:
+                pass
 
         # Return the content that was removed
         return True

tests/integration/test_charm.py

@@ -417,6 +417,15 @@ async def test_provider_get_set_delete_fields(field, value, ops_test: OpsTest):
         is None
     )
 
+    # Delete non-existent field
+    action = await ops_test.model.units.get(leader_name).run_action(
+        "delete-relation-field",
+        **{"relation_id": pytest.second_database_relation.id, "field": "doesnt_exist"},
+    )
+    await action.wait()
+    # Juju2 syntax
+    assert int(action.results["Code"]) == 0
+
 
 @pytest.mark.parametrize(
     "field,value,relation_field",
@@ -502,6 +511,21 @@ async def test_provider_get_set_delete_fields_secrets(
     await action.wait()
     assert not action.results.get("value")
 
+    # Delete non-existent notmal and secret field
+    action = await ops_test.model.units.get(leader_name).run_action(
+        "delete-relation-field",
+        **{"relation_id": pytest.second_database_relation.id, "field": "doesnt_exist"},
+    )
+    await action.wait()
+    assert action.results["return-code"] == 0
+
+    action = await ops_test.model.units.get(leader_name).run_action(
+        "delete-relation-field",
+        **{"relation_id": pytest.second_database_relation.id, "field": "tls-ca"},
+    )
+    await action.wait()
+    assert action.results["return-code"] == 0
+
 
 @pytest.mark.usefixtures("only_with_juju_secrets")
 async def test_provider_deleted_secret_is_removed(ops_test: OpsTest):
@@ -522,7 +546,7 @@ async def test_provider_deleted_secret_is_removed(ops_test: OpsTest):
 
     assert (
         await get_application_relation_data(
-            ops_test, APPLICATION_APP_NAME, SECOND_DATABASE_RELATION_NAME, "tls"
+            ops_test, APPLICATION_APP_NAME, SECOND_DATABASE_RELATION_NAME, "secret-tls"
         )
         is None
     )