Move request sync logic into GoogleConfig (#28227)

* Move request sync logic into GoogleConfig

* Return http status code for request_sync same as cloud

* agent_user_id is not optional for async_sync_entities now

* No need in checking parameter here

* Adjust some things for cloud tests

* Adjust some more stuff for cloud tests

* Drop uneccessary else

* Black required change

* Let async_schedule_google_sync take agent_user_id

* Assert return value on api call

* Test old api key method

* Update homeassistant/components/google_assistant/helpers.py

Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>

homeassistant/components/cloud/google_config.py

@@ -105,7 +105,7 @@ class CloudGoogleConfig(AbstractConfig):
         except ErrorResponse as err:
             _LOGGER.warning("Error reporting state - %s: %s", err.code, err.message)
 
-    async def _async_request_sync_devices(self):
+    async def _async_request_sync_devices(self, agent_user_id: str):
         """Trigger a sync with Google."""
         if self._sync_entities_lock.locked():
             return 200
@@ -143,7 +143,7 @@ class CloudGoogleConfig(AbstractConfig):
 
             # State reporting is reported as a property on entities.
             # So when we change it, we need to sync all entities.
-            await self.async_sync_entities()
+            await self.async_sync_entities(self.agent_user_id)
 
         # If entity prefs are the same or we have filter in config.yaml,
         # don't sync.
@@ -151,7 +151,7 @@ class CloudGoogleConfig(AbstractConfig):
             self._cur_entity_prefs is not prefs.google_entity_configs
             and self._config["filter"].empty_filter
         ):
-            self.async_schedule_google_sync()
+            self.async_schedule_google_sync(self.agent_user_id)
 
         if self.enabled and not self.is_local_sdk_active:
             self.async_enable_local_sdk()
@@ -167,4 +167,4 @@ class CloudGoogleConfig(AbstractConfig):
 
         # Schedule a sync if a change was made to an entity that Google knows about
         if self._should_expose_entity_id(entity_id):
-            await self.async_sync_entities()
+            await self.async_sync_entities(self.agent_user_id)

homeassistant/components/cloud/http_api.py

@@ -174,7 +174,9 @@ class GoogleActionsSyncView(HomeAssistantView):
         """Trigger a Google Actions sync."""
         hass = request.app["hass"]
         cloud: Cloud = hass.data[DOMAIN]
-        status = await cloud.client.google_config.async_sync_entities()
+        status = await cloud.client.google_config.async_sync_entities(
+            cloud.client.google_config.agent_user_id
+        )
         return self.json({}, status_code=status)
 
 

homeassistant/components/google_assistant/__init__.py

@@ -1,11 +1,7 @@
 """Support for Actions on Google Assistant Smart Home Control."""
-import asyncio
 import logging
 from typing import Dict, Any
 
-import aiohttp
-import async_timeout
-
 import voluptuous as vol
 
 # Typing imports
@@ -13,7 +9,6 @@ from homeassistant.core import HomeAssistant, ServiceCall
 
 from homeassistant.const import CONF_NAME
 from homeassistant.helpers import config_validation as cv
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
 from .const import (
     DOMAIN,
@@ -24,7 +19,6 @@ from .const import (
     DEFAULT_EXPOSED_DOMAINS,
     CONF_API_KEY,
     SERVICE_REQUEST_SYNC,
-    REQUEST_SYNC_BASE_URL,
     CONF_ENTITY_CONFIG,
     CONF_EXPOSE,
     CONF_ALIASES,
@@ -99,14 +93,10 @@ CONFIG_SCHEMA = vol.Schema({DOMAIN: GOOGLE_ASSISTANT_SCHEMA}, extra=vol.ALLOW_EX
 async def async_setup(hass: HomeAssistant, yaml_config: Dict[str, Any]):
     """Activate Google Actions component."""
     config = yaml_config.get(DOMAIN, {})
-    api_key = config.get(CONF_API_KEY)
+    google_config = async_register_http(hass, config)
-    async_register_http(hass, config)
 
     async def request_sync_service_handler(call: ServiceCall):
         """Handle request sync service calls."""
-        websession = async_get_clientsession(hass)
-        try:
-            with async_timeout.timeout(15):
         agent_user_id = call.data.get("agent_user_id") or call.context.user_id
 
         if agent_user_id is None:
@@ -115,21 +105,10 @@ async def async_setup(hass: HomeAssistant, yaml_config: Dict[str, Any]):
             )
             return
 
-                res = await websession.post(
+        await google_config.async_sync_entities(agent_user_id)
-                    REQUEST_SYNC_BASE_URL,
-                    params={"key": api_key},
-                    json={"agent_user_id": agent_user_id},
-                )
-                _LOGGER.info("Submitted request_sync request to Google")
-                res.raise_for_status()
-        except aiohttp.ClientResponseError:
-            body = await res.read()
-            _LOGGER.error("request_sync request failed: %d %s", res.status, body)
-        except (asyncio.TimeoutError, aiohttp.ClientError):
-            _LOGGER.error("Could not contact Google for request_sync")
 
-    # Register service only if api key is provided
+    # Register service only if key is provided
-    if api_key is not None:
+    if CONF_API_KEY in config or CONF_SERVICE_ACCOUNT in config:
         hass.services.async_register(
             DOMAIN, SERVICE_REQUEST_SYNC, request_sync_service_handler
         )

homeassistant/components/google_assistant/helpers.py

@@ -41,7 +41,7 @@ class AbstractConfig:
     def __init__(self, hass):
         """Initialize abstract config."""
         self.hass = hass
-        self._google_sync_unsub = None
+        self._google_sync_unsub = {}
         self._local_sdk_active = False
 
     @property
@@ -119,31 +119,29 @@ class AbstractConfig:
             self._unsub_report_state()
             self._unsub_report_state = None
 
-    async def async_sync_entities(self):
+    async def async_sync_entities(self, agent_user_id: str):
         """Sync all entities to Google."""
         # Remove any pending sync
-        if self._google_sync_unsub:
+        self._google_sync_unsub.pop(agent_user_id, lambda: None)()
-            self._google_sync_unsub()
-            self._google_sync_unsub = None
 
-        return await self._async_request_sync_devices()
+        return await self._async_request_sync_devices(agent_user_id)
-
-    async def _schedule_callback(self, _now):
-        """Handle a scheduled sync callback."""
-        self._google_sync_unsub = None
-        await self.async_sync_entities()
 
     @callback
-    def async_schedule_google_sync(self):
+    def async_schedule_google_sync(self, agent_user_id: str):
         """Schedule a sync."""
-        if self._google_sync_unsub:
-            self._google_sync_unsub()
 
-        self._google_sync_unsub = async_call_later(
+        async def _schedule_callback(_now):
-            self.hass, SYNC_DELAY, self._schedule_callback
+            """Handle a scheduled sync callback."""
+            self._google_sync_unsub.pop(agent_user_id, None)
+            await self.async_sync_entities(agent_user_id)
+
+        self._google_sync_unsub.pop(agent_user_id, lambda: None)()
+
+        self._google_sync_unsub[agent_user_id] = async_call_later(
+            self.hass, SYNC_DELAY, _schedule_callback
         )
 
-    async def _async_request_sync_devices(self) -> int:
+    async def _async_request_sync_devices(self, agent_user_id: str) -> int:
         """Trigger a sync with Google.
 
         Return value is the HTTP status code of the sync request.
@@ -165,7 +163,7 @@ class AbstractConfig:
             return
 
         webhook.async_register(
-            self.hass, DOMAIN, "Local Support", webhook_id, self._handle_local_webhook
+            self.hass, DOMAIN, "Local Support", webhook_id, self._handle_local_webhook,
         )
 
         self._local_sdk_active = True

homeassistant/components/google_assistant/http.py

@@ -10,7 +10,7 @@ from aiohttp.web import Request, Response
 
 # Typing imports
 from homeassistant.components.http import HomeAssistantView
-from homeassistant.core import callback, ServiceCall
+from homeassistant.core import callback
 from homeassistant.const import CLOUD_NEVER_EXPOSED_ENTITIES
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.util import dt as dt_util
@@ -27,12 +27,10 @@ from .const import (
     CONF_SERVICE_ACCOUNT,
     CONF_CLIENT_EMAIL,
     CONF_PRIVATE_KEY,
-    DOMAIN,
     HOMEGRAPH_TOKEN_URL,
     HOMEGRAPH_SCOPE,
     REPORT_STATE_BASE_URL,
     REQUEST_SYNC_BASE_URL,
-    SERVICE_REQUEST_SYNC,
 )
 from .smart_home import async_handle_message
 from .helpers import AbstractConfig
@@ -133,6 +131,18 @@ class GoogleConfig(AbstractConfig):
         """If an entity should have 2FA checked."""
         return True
 
+    async def _async_request_sync_devices(self, agent_user_id: str):
+        if CONF_API_KEY in self._config:
+            await self.async_call_homegraph_api_key(
+                REQUEST_SYNC_BASE_URL, {"agentUserId": agent_user_id}
+            )
+        elif CONF_SERVICE_ACCOUNT in self._config:
+            await self.async_call_homegraph_api(
+                REQUEST_SYNC_BASE_URL, {"agentUserId": agent_user_id}
+            )
+        else:
+            _LOGGER.error("No configuration for request_sync available")
+
     async def _async_update_token(self, force=False):
         if CONF_SERVICE_ACCOUNT not in self._config:
             _LOGGER.error("Trying to get homegraph api token without service account")
@@ -151,6 +161,25 @@ class GoogleConfig(AbstractConfig):
             self._access_token = token["access_token"]
             self._access_token_renew = now + timedelta(seconds=token["expires_in"])
 
+    async def async_call_homegraph_api_key(self, url, data):
+        """Call a homegraph api with api key authentication."""
+        websession = async_get_clientsession(self.hass)
+        try:
+            res = await websession.post(
+                url, params={"key": self._config.get(CONF_API_KEY)}, json=data
+            )
+            _LOGGER.debug(
+                "Response on %s with data %s was %s", url, data, await res.text()
+            )
+            res.raise_for_status()
+            return res.status
+        except ClientResponseError as error:
+            _LOGGER.error("Request for %s failed: %d", url, error.status)
+            return error.status
+        except (asyncio.TimeoutError, ClientError):
+            _LOGGER.error("Could not contact %s", url)
+            return 500
+
     async def async_call_homegraph_api(self, url, data):
         """Call a homegraph api with authenticaiton."""
         session = async_get_clientsession(self.hass)
@@ -165,24 +194,26 @@ class GoogleConfig(AbstractConfig):
                     "Response on %s with data %s was %s", url, data, await res.text()
                 )
                 res.raise_for_status()
+                return res.status
 
         try:
             await self._async_update_token()
             try:
-                await _call()
+                return await _call()
             except ClientResponseError as error:
                 if error.status == 401:
                     _LOGGER.warning(
                         "Request for %s unauthorized, renewing token and retrying", url
                     )
                     await self._async_update_token(True)
-                    await _call()
+                    return await _call()
-                else:
                 raise
         except ClientResponseError as error:
             _LOGGER.error("Request for %s failed: %d", url, error.status)
+            return error.status
         except (asyncio.TimeoutError, ClientError):
             _LOGGER.error("Could not contact %s", url)
+            return 500
 
     async def async_report_state(self, message):
         """Send a state report to Google."""
@@ -201,25 +232,7 @@ def async_register_http(hass, cfg):
     hass.http.register_view(GoogleAssistantView(config))
     if config.should_report_state:
         config.async_enable_report_state()
-
+    return config
-    async def request_sync_service_handler(call: ServiceCall):
-        """Handle request sync service calls."""
-        agent_user_id = call.data.get("agent_user_id") or call.context.user_id
-
-        if agent_user_id is None:
-            _LOGGER.warning(
-                "No agent_user_id supplied for request_sync. Call as a user or pass in user id as agent_user_id."
-            )
-            return
-        await config.async_call_homegraph_api(
-            REQUEST_SYNC_BASE_URL, {"agentUserId": agent_user_id}
-        )
-
-    # Register service only if api key is provided
-    if CONF_API_KEY not in cfg and CONF_SERVICE_ACCOUNT in cfg:
-        hass.services.async_register(
-            DOMAIN, SERVICE_REQUEST_SYNC, request_sync_service_handler
-        )
 
 
 class GoogleAssistantView(HomeAssistantView):

tests/components/cloud/test_google_config.py

@@ -12,7 +12,12 @@ from tests.common import mock_coro, async_fire_time_changed
 
 async def test_google_update_report_state(hass, cloud_prefs):
     """Test Google config responds to updating preference."""
-    config = CloudGoogleConfig(hass, GACTIONS_SCHEMA({}), cloud_prefs, None)
+    config = CloudGoogleConfig(
+        hass,
+        GACTIONS_SCHEMA({}),
+        cloud_prefs,
+        Mock(claims={"cognito:username": "abcdefghjkl"}),
+    )
 
     with patch.object(
         config, "async_sync_entities", side_effect=mock_coro
@@ -39,12 +44,17 @@ async def test_sync_entities(aioclient_mock, hass, cloud_prefs):
         ),
     )
 
-    assert await config.async_sync_entities() == 404
+    assert await config.async_sync_entities("user") == 404
 
 
 async def test_google_update_expose_trigger_sync(hass, cloud_prefs):
     """Test Google config responds to updating exposed entities."""
-    config = CloudGoogleConfig(hass, GACTIONS_SCHEMA({}), cloud_prefs, None)
+    config = CloudGoogleConfig(
+        hass,
+        GACTIONS_SCHEMA({}),
+        cloud_prefs,
+        Mock(claims={"cognito:username": "abcdefghjkl"}),
+    )
 
     with patch.object(
         config, "async_sync_entities", side_effect=mock_coro

tests/components/cloud/test_http_api.py

@@ -85,14 +85,18 @@ def mock_cognito():
         yield mock_cog()
 
 
-async def test_google_actions_sync(mock_cognito, cloud_client, aioclient_mock):
+async def test_google_actions_sync(
+    mock_cognito, mock_cloud_login, cloud_client, aioclient_mock
+):
     """Test syncing Google Actions."""
     aioclient_mock.post(GOOGLE_ACTIONS_SYNC_URL)
     req = await cloud_client.post("/api/cloud/google_actions/sync")
     assert req.status == 200
 
 
-async def test_google_actions_sync_fails(mock_cognito, cloud_client, aioclient_mock):
+async def test_google_actions_sync_fails(
+    mock_cognito, mock_cloud_login, cloud_client, aioclient_mock
+):
     """Test syncing Google Actions gone bad."""
     aioclient_mock.post(GOOGLE_ACTIONS_SYNC_URL, status=403)
     req = await cloud_client.post("/api/cloud/google_actions/sync")

tests/components/google_assistant/test_http.py

@@ -106,7 +106,8 @@ async def test_call_homegraph_api(hass, aioclient_mock, hass_storage):
 
         aioclient_mock.post(MOCK_URL, status=200, json={})
 
-        await config.async_call_homegraph_api(MOCK_URL, MOCK_JSON)
+        res = await config.async_call_homegraph_api(MOCK_URL, MOCK_JSON)
+        assert res == 200
 
         assert mock_get_token.call_count == 1
         assert aioclient_mock.call_count == 1
@@ -139,6 +140,34 @@ async def test_call_homegraph_api_retry(hass, aioclient_mock, hass_storage):
         assert call[3] == MOCK_HEADER
 
 
+async def test_call_homegraph_api_key(hass, aioclient_mock, hass_storage):
+    """Test the function to call the homegraph api."""
+    config = GoogleConfig(
+        hass, GOOGLE_ASSISTANT_SCHEMA({"project_id": "1234", "api_key": "dummy_key"})
+    )
+    aioclient_mock.post(MOCK_URL, status=200, json={})
+
+    res = await config.async_call_homegraph_api_key(MOCK_URL, MOCK_JSON)
+    assert res == 200
+    assert aioclient_mock.call_count == 1
+
+    call = aioclient_mock.mock_calls[0]
+    assert call[1].query == {"key": "dummy_key"}
+    assert call[2] == MOCK_JSON
+
+
+async def test_call_homegraph_api_key_fail(hass, aioclient_mock, hass_storage):
+    """Test the function to call the homegraph api."""
+    config = GoogleConfig(
+        hass, GOOGLE_ASSISTANT_SCHEMA({"project_id": "1234", "api_key": "dummy_key"})
+    )
+    aioclient_mock.post(MOCK_URL, status=666, json={})
+
+    res = await config.async_call_homegraph_api_key(MOCK_URL, MOCK_JSON)
+    assert res == 666
+    assert aioclient_mock.call_count == 1
+
+
 async def test_report_state(hass, aioclient_mock, hass_storage):
     """Test the report state function."""
     config = GoogleConfig(hass, DUMMY_CONFIG)