Add Relation.active, and exclude relations that are not active from M…

…odel.relations.

Still need to add unit tests covering the new functionality.

ops/main.py

@@ -144,6 +144,10 @@ def _emit_charm_event(charm: 'ops.charm.CharmBase', event_name: str):
         event_to_emit.emit(*args, **kwargs)
 
 
+def _get_juju_relation_id():
+    return int(os.environ['JUJU_RELATION_ID'].split(':')[-1])
+
+
 def _get_event_args(charm: 'ops.charm.CharmBase',
                     bound_event: 'ops.framework.BoundEvent') -> Tuple[List[Any], Dict[str, Any]]:
     event_type = bound_event.event_type
@@ -183,7 +187,7 @@ def _get_event_args(charm: 'ops.charm.CharmBase',
         return [storage], {}
     elif issubclass(event_type, ops.charm.RelationEvent):
         relation_name = os.environ['JUJU_RELATION']
-        relation_id = int(os.environ['JUJU_RELATION_ID'].split(':')[-1])
+        relation_id = _get_juju_relation_id()
         relation: Optional[ops.model.Relation] = model.get_relation(relation_name, relation_id)
 
     remote_app_name = os.environ.get('JUJU_REMOTE_APP', '')
@@ -386,8 +390,14 @@ def main(charm_class: Type[ops.charm.CharmBase],
     else:
         actions_metadata = None
 
+    # If we are in a RelationBroken event, we want to know which relation is
+    # broken within the model, not only in the event's `.relation` attribute.
+    if os.environ.get("JUJU_DISPATCH_PATH", "").endswith("-relation-broken"):
+        broken_relation_id = _get_juju_relation_id()
+    else:
+        broken_relation_id = None
     meta = CharmMeta.from_yaml(metadata, actions_metadata)
-    model = ops.model.Model(meta, model_backend)
+    model = ops.model.Model(meta, model_backend, broken_relation_id=broken_relation_id)
 
     charm_state_path = charm_dir / CHARM_STATE_FILE
 

ops/model.py

@@ -111,12 +111,14 @@ class Model:
     as ``self.model`` from any class that derives from :class:`Object`.
     """
 
-    def __init__(self, meta: 'ops.charm.CharmMeta', backend: '_ModelBackend'):
+    def __init__(self, meta: 'ops.charm.CharmMeta', backend: '_ModelBackend',
+                 broken_relation_id: Optional[int] = None):
         self._cache = _ModelCache(meta, backend)
         self._backend = backend
         self._unit = self.get_unit(self._backend.unit_name)
         relations: Dict[str, 'ops.RelationMeta'] = meta.relations
-        self._relations = RelationMapping(relations, self.unit, self._backend, self._cache)
+        self._relations = RelationMapping(relations, self.unit, self._backend, self._cache,
+                                          broken_relation_id=broken_relation_id)
         self._config = ConfigData(self._backend)
         resources: Iterable[str] = meta.resources
         self._resources = Resources(list(resources), self._backend)
@@ -147,6 +149,9 @@ def relations(self) -> 'RelationMapping':
 
         Answers the question "what am I currently related to".
         See also :meth:`.get_relation`.
+
+        In a ``relation-broken`` event, the broken relation is excluded from
+        this list.
         """
         return self._relations
 
@@ -796,15 +801,21 @@ def __repr__(self):
 class RelationMapping(Mapping[str, List['Relation']]):
     """Map of relation names to lists of :class:`Relation` instances."""
 
-    def __init__(self, relations_meta: Dict[str, 'ops.RelationMeta'], our_unit: 'Unit',
-                 backend: '_ModelBackend', cache: '_ModelCache'):
+    def __init__(self,
+                 relations_meta: Dict[str,
+                                      'ops.RelationMeta'],
+                 our_unit: 'Unit',
+                 backend: '_ModelBackend',
+                 cache: '_ModelCache',
+                 broken_relation_id: Optional[int]):
         self._peers: Set[str] = set()
         for name, relation_meta in relations_meta.items():
             if relation_meta.role.is_peer():
                 self._peers.add(name)
         self._our_unit = our_unit
         self._backend = backend
         self._cache = cache
+        self._broken_relation_id = broken_relation_id
         self._data: _RelationMapping_Raw = {r: None for r in relations_meta}
 
     def __contains__(self, key: str):
@@ -822,8 +833,10 @@ def __getitem__(self, relation_name: str) -> List['Relation']:
         if not isinstance(relation_list, list):
             relation_list = self._data[relation_name] = []  # type: ignore
             for rid in self._backend.relation_ids(relation_name):
+                if rid == self._broken_relation_id:
+                    continue
                 relation = Relation(relation_name, rid, is_peer,
-                                    self._our_unit, self._backend, self._cache)
+                                    self._our_unit, self._backend, self._cache, True)
                 relation_list.append(relation)
         return relation_list
 
@@ -849,7 +862,7 @@ def _get_unique(self, relation_name: str, relation_id: Optional[int] = None):
                 # The relation may be dead, but it is not forgotten.
                 is_peer = relation_name in self._peers
                 return Relation(relation_name, relation_id, is_peer,
-                                self._our_unit, self._backend, self._cache)
+                                self._our_unit, self._backend, self._cache, False)
         relations = self[relation_name]
         num_related = len(relations)
         self._backend._validate_relation_access(
@@ -1453,13 +1466,20 @@ class Relation:
     This is accessed using, for example, ``Relation.data[unit]['foo']``.
     """
 
+    active: bool
+    """Indicates whether this relation is active.
+
+    This will be ``False`` if the current event is a ``relation-broken`` event
+    assoeciated with this relation."""
+
     def __init__(
             self, relation_name: str, relation_id: int, is_peer: bool, our_unit: Unit,
-            backend: '_ModelBackend', cache: '_ModelCache'):
+            backend: '_ModelBackend', cache: '_ModelCache', active: bool):
         self.name = relation_name
         self.id = relation_id
         self.app: Optional[Application] = None
         self.units: Set[Unit] = set()
+        self.active = active
 
         if is_peer:
             # For peer relations, both the remote and the local app are the same.
@@ -1474,7 +1494,7 @@ def __init__(
                     self.app = unit.app
         except RelationNotFoundError:
             # If the relation is dead, just treat it as if it has no remote units.
-            pass
+            self.active = False
 
         # If we didn't get the remote app via our_unit.app or the units list,
         # look it up via JUJU_REMOTE_APP or "relation-list --app".

test/test_model.py

@@ -3349,16 +3349,16 @@ def test_grant(self):
         meta = ops.CharmMeta()
         cache = ops.model._ModelCache(meta, backend)
         unit = ops.Unit('test', meta, backend, cache)
-        rel123 = ops.Relation('test', 123, True, unit, backend, cache)
-        rel234 = ops.Relation('test', 234, True, unit, backend, cache)
+        rel123 = ops.Relation('test', 123, True, unit, backend, cache, True)
+        rel234 = ops.Relation('test', 234, True, unit, backend, cache, True)
         secret.grant(rel123)
         unit = ops.Unit('app/0', meta, backend, cache)
         secret.grant(rel234, unit=unit)
 
         # If secret doesn't have an ID, we'll run secret-info-get to fetch it
         secret = self.make_secret(label='y')
         self.assertIsNone(secret.id)
-        rel345 = ops.Relation('test', 345, True, unit, backend, cache)
+        rel345 = ops.Relation('test', 345, True, unit, backend, cache, True)
         secret.grant(rel345)
         self.assertEqual(secret.id, 'secret:z')
 
@@ -3379,16 +3379,19 @@ def test_revoke(self):
 
         secret = self.make_secret(id='x')
         unit = ops.Unit('test', ops.CharmMeta(), self.model._backend, self.model._cache)
-        rel123 = ops.Relation('test', 123, True, unit, self.model._backend, self.model._cache)
-        rel234 = ops.Relation('test', 234, True, unit, self.model._backend, self.model._cache)
+        rel123 = ops.Relation('test', 123, True, unit, self.model._backend, self.model._cache,
+                              True)
+        rel234 = ops.Relation('test', 234, True, unit, self.model._backend, self.model._cache,
+                              True)
         secret.revoke(rel123)
         unit = ops.Unit('app/0', ops.CharmMeta(), self.model._backend, self.model._cache)
         secret.revoke(rel234, unit=unit)
 
         # If secret doesn't have an ID, we'll run secret-info-get to fetch it
         secret = self.make_secret(label='y')
         self.assertIsNone(secret.id)
-        rel345 = ops.Relation('test', 345, True, unit, self.model._backend, self.model._cache)
+        rel345 = ops.Relation('test', 345, True, unit, self.model._backend, self.model._cache,
+                              True)
         secret.revoke(rel345)
         self.assertEqual(secret.id, 'secret:z')