From 522d54e43655830fdb99589721e8a2f1af2a743a Mon Sep 17 00:00:00 2001
From: Adam Dyess <adam.dyess@canonical.com>
Date: Thu, 23 May 2024 23:30:21 -0500
Subject: [PATCH] Prevent refresh by revision when the snap is already on that
 revision (#121)

---
 lib/charms/operator_libs_linux/v2/snap.py |  9 ++++++---
 tests/unit/test_snap.py                   | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py
index ef42677..6d4dc38 100644
--- a/lib/charms/operator_libs_linux/v2/snap.py
+++ b/lib/charms/operator_libs_linux/v2/snap.py
@@ -83,7 +83,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 5
+LIBPATCH = 6
 
 
 # Regex to locate 7-bit C1 ANSI sequences
@@ -584,13 +584,16 @@ def ensure(
                     "Installing snap %s, revision %s, tracking %s", self._name, revision, channel
                 )
                 self._install(channel, cohort, revision)
-            else:
+                logger.info("The snap installation completed successfully")
+            elif revision is None or revision != self._revision:
                 # The snap is installed, but we are changing it (e.g., switching channels).
                 logger.info(
                     "Refreshing snap %s, revision %s, tracking %s", self._name, revision, channel
                 )
                 self._refresh(channel=channel, cohort=cohort, revision=revision, devmode=devmode)
-            logger.info("The snap installation completed successfully")
+                logger.info("The snap refresh completed successfully")
+            else:
+                logger.info("Refresh of snap %s was unnecessary", self._name)
 
         self._update_snap_apps()
         self._state = state
diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py
index e1e1a82..94875a1 100644
--- a/tests/unit/test_snap.py
+++ b/tests/unit/test_snap.py
@@ -543,6 +543,25 @@ def test_cohort(self, mock_subprocess):
             universal_newlines=True,
         )
 
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
+    def test_revision_doesnt_refresh(self, mock_check_output):
+        snap.add("curl", revision="233", cohort="+")
+        mock_check_output.assert_called_with(
+            [
+                "snap",
+                "install",
+                "curl",
+                '--revision="233"',
+                '--cohort="+"',
+            ],
+            universal_newlines=True,
+        )
+
+        mock_check_output.reset_mock()
+        # Ensure that calling refresh with the same revision doesn't subprocess out.
+        snap.ensure("curl", "latest", classic=True, revision="233", cohort="+")
+        mock_check_output.assert_not_called()
+
     @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_can_ensure_states(self, mock_subprocess):
         mock_subprocess.return_value = 0