fix(refresh): bug with revisions

We are incorrectly refreshing charms. The current method queries the
controller for the charm's current origin and charm url

In pylibjuju, we then just overwrite the appropriate fields. However,
this leaves behind id_ and hash_

This means when we later refresh, we are always returned the most recent
revision by charmhub, no matter what revision we specify.

The way we handle this in the Juju CLI is by reconstructing the charm
origin using MakeOrigin:
https://github.com/juju/juju/blob/a03ade4d358b1b0f135374b15b1bcd6b46d0ba0f/cmd/juju/application/utils/origin.go#L20-L75

This ensures only the fields we expect are present.

Replicate this method, ensuring we behave the same as the CLI.

Fixes: #1057

juju/application.py

@@ -18,6 +18,7 @@
 from .status import derive_status
 from .url import URL
 from .utils import block_until
+from .version import DEFAULT_ARCHITECTURE
 
 log = logging.getLogger(__name__)
 
@@ -691,12 +692,6 @@ async def refresh(
         if charm_url_origin_result.error is not None:
             err = charm_url_origin_result.error
             raise JujuError(f'{err.code} : {err.message}')
-        origin = charm_url_origin_result.charm_origin
-
-        if path is not None or (switch is not None and is_local_charm(switch)):
-            await self.local_refresh(origin, force, force_series,
-                                     force_units, path or switch, resources)
-            return
 
         # If switch is not None at this point, that means it's a switch to a store charm
         charm_url = switch or charm_url_origin_result.url
@@ -706,19 +701,24 @@ async def refresh(
         if parsed_url.schema is None:
             raise JujuError(f'A ch: or cs: schema is required for application refresh, given : {str(parsed_url)}')
 
-        if revision is not None:
-            origin.revision = revision
+        current_origin = charm_url_origin_result.charm_origin
+        if path is not None or (switch is not None and is_local_charm(switch)):
+            await self.local_refresh(current_origin, force, force_series,
+                                     force_units, path or switch, resources)
+            return
 
-        # Make the source-specific changes to the origin/channel/url
-        # (and also get the resources necessary to deploy the (destination) charm -- for later)
-        origin.source = Source.CHARM_HUB.value
-        if channel:
+        ch = None
+        if channel is not None:
             ch = Channel.parse(channel).normalize()
-            origin.risk = ch.risk
-            origin.track = ch.track
 
-        charmhub = self.model.charmhub
-        charm_resources = await charmhub.list_resources(charm_name)
+        origin = client.CharmOrigin(
+            source=str(Source.CHARM_HUB),
+            risk=ch.risk if ch else current_origin.risk,
+            track=ch.track if ch else current_origin.track,
+            revision=revision or current_origin.revision,
+            base=current_origin.base,
+            architecture=current_origin.architecture or DEFAULT_ARCHITECTURE,
+        )
 
         # Resolve the given charm URLs with an optionally specified preferred channel.
         # Channel provided via CharmOrigin.
@@ -761,8 +761,7 @@ async def refresh(
             else:
                 _arg_res_filenames[res] = filename_or_rev
 
-        # Already prepped the charm_resources
-        # Now get the existing resources from the ResourcesFacade
+        # Get the existing resources from the ResourcesFacade
         request_data = [client.Entity(self.tag)]
         resources_facade = client.ResourcesFacade.from_connection(self.connection)
         response = await resources_facade.ListResources(entities=request_data)
@@ -771,6 +770,9 @@ async def refresh(
             for resource in response.results[0].resources
         }
 
+        charmhub = self.model.charmhub
+        charm_resources = await charmhub.list_resources(charm_name)
+
         # Compute the difference btw resources needed and the existing resources
         resources_to_update = []
         for resource in charm_resources:

tests/integration/test_application.py

@@ -286,6 +286,18 @@ async def test_local_refresh():
                                             base=client.Base("20.04", "ubuntu"))
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_refresh_revision():
+    async with base.CleanModel() as model:
+        app = await model.deploy('juju-qa-test', channel="latest/stable", revision=23)
+        # NOTE: juju-qa-test revision 26 has been releases to this channel
+        await app.refresh(revision=25)
+
+        charm_url = URL.parse(app.data['charm-url'])
+        assert charm_url.revision == 25
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_trusted():