Properly handle the case when a group includes itself. (#8398)

* Properly handle the case when a group includes itself. * Fix lint
home-assistant · Jul 8, 2017 · c5bf4fe · c5bf4fe
1 parent 57c5ed3
commit c5bf4fe
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 25 deletions.
diff --git a/homeassistant/components/group.py b/homeassistant/components/group.py
@@ -184,7 +184,6 @@ def expand_entity_ids(hass, entity_ids):
     Async friendly.
     """
     found_ids = []
-
     for entity_id in entity_ids:
         if not isinstance(entity_id, str):
             continue
@@ -196,9 +195,13 @@ def expand_entity_ids(hass, entity_ids):
             domain, _ = ha.split_entity_id(entity_id)
 
             if domain == DOMAIN:
+                child_entities = get_entity_ids(hass, entity_id)
+                if entity_id in child_entities:
+                    child_entities = list(child_entities)
+                    child_entities.remove(entity_id)
                 found_ids.extend(
                     ent_id for ent_id
-                    in expand_entity_ids(hass, get_entity_ids(hass, entity_id))
+                    in expand_entity_ids(hass, child_entities)
                     if ent_id not in found_ids)
 
             else:
@@ -223,7 +226,6 @@ def get_entity_ids(hass, entity_id, domain_filter=None):
         return []
 
     entity_ids = group.attributes[ATTR_ENTITY_ID]
-
     if not domain_filter:
         return entity_ids
 

diff --git a/tests/components/test_group.py b/tests/components/test_group.py
@@ -150,6 +150,20 @@ def test_expand_entity_ids_does_not_return_duplicates(self):
             sorted(group.expand_entity_ids(
                 self.hass, ['light.bowl', test_group.entity_id])))
 
+    def test_expand_entity_ids_recursive(self):
+        """Test expand_entity_ids method with a group that contains itself."""
+        self.hass.states.set('light.Bowl', STATE_ON)
+        self.hass.states.set('light.Ceiling', STATE_OFF)
+        test_group = group.Group.create_group(
+            self.hass,
+            'init_group',
+            ['light.Bowl', 'light.Ceiling', 'group.init_group'],
+            False)
+
+        self.assertEqual(sorted(['light.ceiling', 'light.bowl']),
+                         sorted(group.expand_entity_ids(
+                             self.hass, [test_group.entity_id])))
+
     def test_expand_entity_ids_ignores_non_strings(self):
         """Test that non string elements in lists are ignored."""
         self.assertEqual([], group.expand_entity_ids(self.hass, [5, True]))
@@ -226,11 +240,11 @@ def test_setup(self):
 
         group_conf = OrderedDict()
         group_conf['second_group'] = {
-                        'entities': 'light.Bowl, ' + test_group.entity_id,
-                        'icon': 'mdi:work',
-                        'view': True,
-                        'control': 'hidden',
-                    }
+            'entities': 'light.Bowl, ' + test_group.entity_id,
+            'icon': 'mdi:work',
+            'view': True,
+            'control': 'hidden',
+        }
         group_conf['test_group'] = 'hello.world,sensor.happy'
         group_conf['empty_group'] = {'name': 'Empty Group', 'entities': None}
 
@@ -275,8 +289,8 @@ def test_expand_entity_ids_expands_nested_groups(self):
             self.hass, 'light', ['light.test_1', 'light.test_2'])
         group.Group.create_group(
             self.hass, 'switch', ['switch.test_1', 'switch.test_2'])
-        group.Group.create_group(self.hass, 'group_of_groups', ['group.light',
-                                 'group.switch'])
+        group.Group.create_group(
+            self.hass, 'group_of_groups', ['group.light', 'group.switch'])
 
         self.assertEqual(
             ['light.test_1', 'light.test_2', 'switch.test_1', 'switch.test_2'],
@@ -325,27 +339,26 @@ def test_group_updated_after_device_tracker_zone_change(self):
     def test_reloading_groups(self):
         """Test reloading the group config."""
         assert setup_component(self.hass, 'group', {'group': {
-                    'second_group': {
-                        'entities': 'light.Bowl',
-                        'icon': 'mdi:work',
-                        'view': True,
-                    },
-                    'test_group': 'hello.world,sensor.happy',
-                    'empty_group': {'name': 'Empty Group', 'entities': None},
-                }
-            })
+            'second_group': {
+                'entities': 'light.Bowl',
+                'icon': 'mdi:work',
+                'view': True,
+            },
+            'test_group': 'hello.world,sensor.happy',
+            'empty_group': {'name': 'Empty Group', 'entities': None},
+        }})
 
         assert sorted(self.hass.states.entity_ids()) == \
             ['group.empty_group', 'group.second_group', 'group.test_group']
         assert self.hass.bus.listeners['state_changed'] == 3
 
         with patch('homeassistant.config.load_yaml_config_file', return_value={
-                'group': {
-                    'hello': {
-                        'entities': 'light.Bowl',
-                        'icon': 'mdi:work',
-                        'view': True,
-                    }}}):
+            'group': {
+                'hello': {
+                    'entities': 'light.Bowl',
+                    'icon': 'mdi:work',
+                    'view': True,
+                }}}):
             group.reload(self.hass)
             self.hass.block_till_done()
 
@@ -395,6 +408,7 @@ def test_service_group_services(hass):
     assert hass.services.has_service('group', group.SERVICE_REMOVE)
 
 
+# pylint: disable=invalid-name
 @asyncio.coroutine
 def test_service_group_set_group_remove_group(hass):
     """Check if service are available."""