Refactor and cleanup code & test for application.refresh

Author: cderici

juju/application.py

@@ -664,12 +664,13 @@ async def refresh(
             charmhub = self.model.charmhub
             charm_resources = await charmhub.list_resources(charm_name)
 
-            res = await app_facade.GetCharmURLOrigin(application=charm_name)
+            charm_url_origin_result = await app_facade.GetCharmURLOrigin(application=charm_name)
 
-            if res.error is not None:
-                raise JujuError(f'{res.code} : {res.message}')
-            charm_url = res.url
-            origin = res.charm_origin
+            if charm_url_origin_result.error is not None:
+                err = charm_url_origin_result.error
+                raise JujuError(f'{err.code} : {err.message}')
+            charm_url = charm_url_origin_result.url
+            origin = charm_url_origin_result.charm_origin
 
             if channel:
                 ch = Channel.parse(channel).normalize()
@@ -685,14 +686,11 @@ async def refresh(
             resolved_charm = resolved_charm_with_channel_results.results[0]
 
             if resolved_charm.error is not None:
-                raise JujuError(f'{res.code} : {res.message}')
+                err = resolved_charm.error
+                raise JujuError(f'{err.code} : {err.message}')
             dest_origin = resolved_charm.charm_origin
             charm_url = resolved_charm.url
 
-            await charms_facade.AddCharm(url=charm_url,
-                                         force=force,
-                                         charm_origin=dest_origin)
-
         else:
             charms_facade = client.CharmsFacade.from_connection(self.connection)
             charmstore = self.model.charmstore
@@ -714,14 +712,19 @@ async def refresh(
                 raise JujuError('already running charm "%s"' % charm_url)
 
             dest_origin = client.CharmOrigin(source="charm-store", risk=channel)
-            # Update charm
-            await charms_facade.AddCharm(url=charm_url,
-                                         force=force,
-                                         charm_origin=dest_origin)
+
             if not charmstore_entity:
                 charmstore_entity = await charmstore.entity(charm_url, channel=channel)
             charm_resources = charmstore_entity['Meta']['resources']
 
+        # Add the charm with the new origin
+        charm_origin_result = await charms_facade.AddCharm(url=charm_url,
+                                                           force=force,
+                                                           charm_origin=dest_origin)
+        if charm_origin_result.error is not None:
+            err = charm_origin_result.error
+            raise JujuError(f'{err.code} : {err.message}')
+
         # Update resources
         request_data = [client.Entity(self.tag)]
         response = await resources_facade.ListResources(entities=request_data)
@@ -744,7 +747,6 @@ async def refresh(
                     fingerprint=resource.get('Fingerprint', resource.get('fingerprint')),
                     name=resource.get('Name', resource.get('name')),
                     path=resource.get('Path', resource.get('filename')),
-                    # revision=-1,
                     revision=resource.get('Revision', resource.get('revision', -1)),
                     size=resource.get('Size', resource.get('size')),
                     type_=resource.get('Type', resource.get('type')),

tests/integration/test_application.py

@@ -174,10 +174,11 @@ async def test_upgrade_charm_channel(event_loop):
 async def test_upgrade_charm_switch_channel(event_loop):
     # Note for future:
     # This test requires a charm that has different
-    # revisions in channels 'stable' and 'candidate'.
-    # Currently, we use mongodb, but eventually
-    # (when the 'candidate' moves to 'stable') this test
+    # revisions for different channels/risks.
+    # Currently, we use juju-qa-test, but eventually
+    # (when the 'edge' moves to 'stable') this test
     # will be testing nothing (if not failing).
+    # So checks are in place for that.
 
     async with base.CleanModel() as model:
         app = await model.deploy('juju-qa-test', channel='2.0/stable')
@@ -187,7 +188,7 @@ async def test_upgrade_charm_switch_channel(event_loop):
         assert Schema.CHARM_HUB.matches(charm_url.schema)
         still22 = False
         try:
-            assert charm_url.revision == 76
+            assert charm_url.revision == 22
             still22 = True
         except AssertionError:
             logger.warning("Charm used in test_upgrade_charm_switch_channel "
@@ -198,6 +199,7 @@ async def test_upgrade_charm_switch_channel(event_loop):
 
         if still22:
             try:
+                charm_url = URL.parse(app.data['charm-url'])
                 assert charm_url.revision == 23
             except AssertionError:
                 raise errors.JujuError("Either the upgrade has failed, or the used charm moved "