Clean up code

src/charm.py

@@ -157,6 +157,17 @@ def _hydra_service_is_created(self) -> bool:
 
  return True
 
+ @property
+ def _hydra_service_is_running(self) -> bool:
+ if not self._container.can_connect():
+ return False
+
+ try:
+ service = self._container.get_service(self._container_name)
+ except (ModelError, RuntimeError):
+ return False
+ return service.is_running()
+
  def _render_conf_file(self) -> None:
  """Render the Hydra configuration file."""
  with open("templates/hydra.yaml.j2", "r") as file:
@@ -353,11 +364,7 @@ def _on_client_created(self, event: ClientCreatedEvent) -> None:
  if not self.unit.is_leader():
  return
 
- if not self._container.can_connect():
- event.defer()
- return
-
- if not self._hydra_service_is_created:
+ if not self._hydra_service_is_running:
  event.defer()
  return
 
@@ -383,11 +390,7 @@ def _on_client_changed(self, event: ClientChangedEvent) -> None:
  if not self.unit.is_leader():
  return
 
- if not self._container.can_connect():
- event.defer()
- return
-
- if not self._hydra_service_is_created:
+ if not self._hydra_service_is_running:
  event.defer()
  return
 

tests/unit/conftest.py

@@ -1,7 +1,7 @@
 # Copyright 2023 Canonical Ltd.
 # See LICENSE file for licensing details.
 
-import json
+from typing import Generator
 
 import pytest
 from ops.testing import Harness
@@ -25,26 +25,36 @@ def mocked_kubernetes_service_patcher(mocker):
  yield mocked_service_patcher
 
 
+@pytest.fixture()
+def mocked_hydra_is_running(mocker) -> Generator:
+ yield mocker.patch("charm.HydraCharm._hydra_service_is_running", return_value=True)
+
+
 @pytest.fixture()
 def mocked_sql_migration(mocker):
  mocked_sql_migration = mocker.patch("charm.HydraCharm._run_sql_migration")
  yield mocked_sql_migration
 
 
 @pytest.fixture()
-def mocked_hydra_cli(mocker):
- mock = mocker.patch("charm.HydraCLI._run_cmd")
- mock.return_value = ("{}", None)
+def mocked_create_client(mocker):
+ mock = mocker.patch("charm.HydraCLI.create_client")
+ mock.return_value = {"client_id": "client_id", "client_secret": "client_secret"}
+ yield mock
+
+
+@pytest.fixture()
+def mocked_update_client(mocker):
+ mock = mocker.patch("charm.HydraCLI.update_client")
+ mock.return_value = {"client_id": "client_id", "client_secret": "client_secret"}
  yield mock
 
 
 @pytest.fixture()
-def mocked_create_client(mocked_hydra_cli):
- mocked_hydra_cli.return_value = (
- json.dumps({"client_id": "client_id", "client_secret": "client_secret"}),
- None,
- )
- yield mocked_hydra_cli
+def mocked_delete_client(mocker):
+ mock = mocker.patch("charm.HydraCLI.delete_client")
+ mock.return_value = "client_id"
+ yield mock
 
 
 @pytest.fixture()

tests/unit/test_charm.py

@@ -414,11 +414,14 @@ def test_provider_info_called_when_oauth_relation_then_ingress(
 
 
 def test_client_created_event(
- harness: Harness, mocked_create_client: MagicMock, mocked_set_client_credentials: MagicMock
+ harness: Harness,
+ mocked_create_client: MagicMock,
+ mocked_set_client_credentials: MagicMock,
+ mocked_hydra_is_running: MagicMock,
 ) -> None:
  harness.set_can_connect(CONTAINER_NAME, True)
  harness.charm.on.hydra_pebble_ready.emit(CONTAINER_NAME)
- client_credentials = json.loads(mocked_create_client.return_value[0])
+ client_credentials = mocked_create_client.return_value
 
  relation_id, _ = setup_oauth_relation(harness)
  harness.charm.oauth.on.client_created.emit(relation_id=relation_id, **CLIENT_CONFIG)
@@ -452,7 +455,10 @@ def test_client_created_event_when_no_service(
 
 
 def test_client_created_event_when_exec_error(
- harness: Harness, mocked_create_client: MagicMock, caplog: pytest.LogCaptureFixture
+ harness: Harness,
+ mocked_create_client: MagicMock,
+ mocked_hydra_is_running: MagicMock,
+ caplog: pytest.LogCaptureFixture,
 ) -> None:
  caplog.set_level(logging.ERROR)
  harness.set_can_connect(CONTAINER_NAME, True)
@@ -470,7 +476,10 @@ def test_client_created_event_when_exec_error(
 
 
 def test_client_created_event_when_error(
- harness: Harness, mocked_create_client: MagicMock, caplog: pytest.LogCaptureFixture
+ harness: Harness,
+ mocked_create_client: MagicMock,
+ mocked_hydra_is_running: MagicMock,
+ caplog: pytest.LogCaptureFixture,
 ) -> None:
  caplog.set_level(logging.ERROR)
  harness.set_can_connect(CONTAINER_NAME, True)
@@ -487,7 +496,9 @@ def test_client_created_event_when_error(
  )
 
 
-def test_client_changed_event(harness: Harness, mocked_hydra_cli: MagicMock) -> None:
+def test_client_changed_event(
+ harness: Harness, mocked_update_client: MagicMock, mocked_hydra_is_running: MagicMock
+) -> None:
  harness.set_can_connect(CONTAINER_NAME, True)
  harness.charm.on.hydra_pebble_ready.emit(CONTAINER_NAME)
 
@@ -496,11 +507,11 @@ def test_client_changed_event(harness: Harness, mocked_hydra_cli: MagicMock) ->
  relation_id=relation_id, client_id="client_id", **CLIENT_CONFIG
  )
 
- assert mocked_hydra_cli.called
+ assert mocked_update_client.called
 
 
 def test_client_changed_event_when_cannot_connect(
- harness: Harness, mocked_hydra_cli: MagicMock
+ harness: Harness, mocked_update_client: MagicMock
 ) -> None:
  harness.set_can_connect(CONTAINER_NAME, False)
 
@@ -509,11 +520,11 @@ def test_client_changed_event_when_cannot_connect(
  relation_id=relation_id, client_id="client_id", **CLIENT_CONFIG
  )
 
- assert not mocked_hydra_cli.called
+ assert not mocked_update_client.called
 
 
 def test_client_changed_event_when_no_service(
- harness: Harness, mocked_hydra_cli: MagicMock
+ harness: Harness, mocked_update_client: MagicMock
 ) -> None:
  harness.set_can_connect(CONTAINER_NAME, True)
 
@@ -522,19 +533,22 @@ def test_client_changed_event_when_no_service(
  relation_id=relation_id, client_id="client_id", **CLIENT_CONFIG
  )
 
- assert not mocked_hydra_cli.called
+ assert not mocked_update_client.called
 
 
 def test_client_changed_event_when_exec_error(
- harness: Harness, mocked_hydra_cli: MagicMock, caplog: pytest.LogCaptureFixture
+ harness: Harness,
+ mocked_update_client: MagicMock,
+ mocked_hydra_is_running: MagicMock,
+ caplog: pytest.LogCaptureFixture,
 ) -> None:
  caplog.set_level(logging.ERROR)
  harness.set_can_connect(CONTAINER_NAME, True)
  harness.charm.on.hydra_pebble_ready.emit(CONTAINER_NAME)
  err = ExecError(
  command=["hydra", "create", "client", "1234"], exit_code=1, stdout="Out", stderr="Error"
  )
- mocked_hydra_cli.side_effect = err
+ mocked_update_client.side_effect = err
 
  relation_id, _ = setup_oauth_relation(harness)
  harness.charm.oauth.on.client_changed.emit(
@@ -546,13 +560,16 @@ def test_client_changed_event_when_exec_error(
 
 
 def test_client_changed_event_when_error(
- harness: Harness, mocked_hydra_cli: MagicMock, caplog: pytest.LogCaptureFixture
+ harness: Harness,
+ mocked_update_client: MagicMock,
+ mocked_hydra_is_running: MagicMock,
+ caplog: pytest.LogCaptureFixture,
 ) -> None:
  caplog.set_level(logging.ERROR)
  harness.set_can_connect(CONTAINER_NAME, True)
  harness.charm.on.hydra_pebble_ready.emit(CONTAINER_NAME)
  err = Error("Some error")
- mocked_hydra_cli.side_effect = err
+ mocked_update_client.side_effect = err
 
  relation_id, _ = setup_oauth_relation(harness)
  harness.charm.oauth.on.client_changed.emit(

tests/unit/test_oauth_provider.py

@@ -146,8 +146,7 @@ def test_client_changed(harness: Harness) -> None:
  )
 
  assert any(
- isinstance(e, ClientChangedEvent)
- and e.redirect_uri == redirect_uri
+ isinstance(e, ClientChangedEvent) and e.redirect_uri == redirect_uri
  for e in harness.charm.events
  )