Repository: juju/python-libjuju
Issue: [Bug]: charm upgrade does not work

### Description

 when I try to run the below code I got the following error.

```
await model.connect()
app = model.applications["keystone"]
await app.upgrade_charm(channel="victoria/stable")
```
`juju.errors.JujuError: while adding pending resource info for "policyd-override": bad resource metadata: bad info: bad metadata: resource missing filename`

Hower below code runs succesfully. What could be the problem?
```
juju refresh --channel victoria/stable  keystone
```

[log](https://pastebin.canonical.com/p/XPDcg26jTV/) , [code](https://pastebin.canonical.com/p/6hkr32qbxs/), [bundle](https://pastebin.canonical.com/p/qVHybXyTvP/), [thread](https://chat.charmhub.io/charmhub/pl/d4bcykjbajre9ktr9pmj117dgy)


juju client: 2.9.43-ubuntu-amd64
controller: 2.9.42, python juju==2.9.42.4

@cderici 


### Urgency

Blocker for our release

### Python-libjuju version

2.9.42.4

### Juju version

2.9.43-ubuntu-amd64

### Reproduce / Test

```python
import asyncio
import logging

from juju.model import Model

JUJU_MAX_FRAME_SIZE = 2**30


def setup_logging(log_level: str = "DEBUG") -> None:
    log_formatter = logging.Formatter(
        fmt="%(asctime)s [%(levelname)s] %(message)s", datefmt="%Y-%m-%d %H:%M:%S"
    )
    root_logger = logging.getLogger()
    root_logger.setLevel(log_level)
    if not root_logger.hasHandlers():
        console_handler = logging.StreamHandler()
        console_handler.setFormatter(log_formatter)
        root_logger.addHandler(console_handler)


async def update():
    model = Model(max_frame_size=JUJU_MAX_FRAME_SIZE)
    await model.connect()
    app = model.applications["keystone"]
    await app.upgrade_charm(channel="victoria/stable")


setup_logging()
loop = asyncio.new_event_loop()
loop.run_until_complete(update())
```


Changed files:
juju/application.py, tests/unit/test_utils.py, juju/utils.py, tests/integration/test_application.py

diff --git a/juju/application.py b/juju/application.py
index 4e8952a..2979871 100644
--- a/juju/application.py
+++ b/juju/application.py
@@ -740,11 +740,10 @@ class Application(model.ModelEntity):
         }
 
         # Compute the difference btw resources needed and the existing resources
-        resources_to_update = [
-            resource for resource in charm_resources
-            if resource.get('Name', resource.get('name')) not in existing_resources or
-            existing_resources[resource.get('Name', resource.get('name'))].origin != 'upload'
-        ]
+        resources_to_update = []
+        for resource in charm_resources:
+            if utils.should_upgrade_resource(resource, existing_resources):
+                resources_to_update.append(resource)
 
         # Update the resources
         if resources_to_update:
diff --git a/juju/utils.py b/juju/utils.py
index abb98ea..dfc442c 100644
--- a/juju/utils.py
+++ b/juju/utils.py
@@ -389,3 +389,30 @@ def base_channel_to_series(channel):
     :return: str series (e.g. focal)
     """
     return get_version_series(origin.Channel.parse(channel).track)
+
+
+def should_upgrade_resource(available_resource, existing_resources):
+    """Called in the context of upgrade_charm. Given a resource R, takes a look at the resources we
+    already have and decides if we need to refresh R.
+
+    :param dict[str] available_resource: The dict representing the client.Resource coming from the
+    charmhub api. We're considering if we need to refresh this during upgrade_charm.
+    :param dict[str] existing_resources: The dict coming from resources_facade.ListResources
+    representing the resources of the currently deployed charm.
+
+    :result bool: The decision to refresh the given resource
+    """
+    # should upgrade resource?
+    res_name = available_resource.get('Name', available_resource.get('name'))
+    # no, if it's upload
+    if existing_resources[res_name].origin == 'upload':
+        return False
+
+    # no, if we already have it (and upstream doesn't have a newer res available)
+    if res_name in existing_resources:
+        available_rev = available_resource.get('Revision', available_resource.get('revision', -1))
+        u_fields = existing_resources[res_name].unknown_fields
+        existing_rev = u_fields.get('Revision', u_fields.get('revision', -1))
+        if existing_rev >= available_rev:
+            return False
+    return True
diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py
index 6ba8e6a..4d5e4b3 100644
--- a/tests/integration/test_application.py
+++ b/tests/integration/test_application.py
@@ -226,6 +226,17 @@ async def test_upgrade_charm_resource(event_loop):
         assert app.units[0].workload_status_message == expected_message
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_upgrade_charm_resource_same_rev_no_update(event_loop):
+    async with base.CleanModel() as model:
+        app = await model.deploy('keystone', channel='victoria/stable')
+        ress = await app.get_resources()
+        await app.refresh(channel='ussuri/stable')
+        ress2 = await app.get_resources()
+        assert ress['policyd-override'].fingerprint == ress2['policyd-override'].fingerprint
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_trusted(event_loop):
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 6a17339..880bf17 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -1,14 +1,85 @@
 import unittest
 
-from juju.utils import juju_config_dir, juju_ssh_key_paths
+from juju import utils
+from juju.client import client
 
 
 class TestDirResolve(unittest.TestCase):
     def test_config_dir(self):
-        config_dir = juju_config_dir()
+        config_dir = utils.juju_config_dir()
         assert 'local/share/juju' in config_dir
 
     def test_juju_ssh_key_paths(self):
-        public, private = juju_ssh_key_paths()
+        public, private = utils.juju_ssh_key_paths()
         assert public.endswith('ssh/juju_id_rsa.pub')
         assert private.endswith('ssh/juju_id_rsa')
+
+
+class TestShouldUpgradeResource(unittest.TestCase):
+    def test_should_upgrade_resource_no_same_rev(self):
+        # fields are trimmed for readability
+        res = {'created-at': '2019-10-24T20:45:19.201000',
+               'description': 'The policy.d overrides file',
+               'download': {'hash-sha-256': 'e3b0c4', 'hash-sha-384': '38b060a751ac914898b95b',
+                            'hash-sha-512': 'cf83e1357eef1a538327af927da3e',
+                            'hash-sha3-384': '0c63a75b1bbed1e058d5f004',
+                            'size': 0,
+                            'url': 'https://api.charmhub.io/api/v1/resMGU0L516cGTTwNam.policyd-override_0'},
+               'filename': 'policyd-override.zip', 'name': 'policyd-override', 'revision': 0, 'type': 'file'}
+
+        existing = {
+            'policyd-override':
+                client.Resource(charmresource=None,
+                                application='keystone', id_='keystone/policyd-override', pending_id='',
+                                timestamp='0001-01-01T00:00:00Z', username='', name='policyd-override',
+                                origin='store', type='file', path='policyd-override.zip',
+                                description='The policy.doverrides file',
+                                revision=0, fingerprint='OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb',
+                                size=0)}
+        assert not utils.should_upgrade_resource(res, existing)
+
+    def test_should_upgrade_resource_no_local_upload(self):
+        # fields are trimmed for readability
+        res = {'created-at': '2019-10-24T20:45:19.201000',
+               'description': 'The policy.d overrides file',
+               'download': {'hash-sha-256': 'e3b0c4', 'hash-sha-384': '38b060a751ac914898b95b',
+                            'hash-sha-512': 'cf83e1357eef1a538327af927da3e',
+                            'hash-sha3-384': '0c63a75b1bbed1e058d5f004',
+                            'size': 0,
+                            'url': 'https://api.charmhub.io/api/v1/resMGU0L516cGTTwNam.policyd-override_0'},
+               'filename': 'policyd-override.zip', 'name': 'local_res', 'revision': 0,
+               'type': 'file'}
+
+        existing = {
+            'local_res':
+                client.Resource(charmresource=None,
+                                application='keystone', id_='keystone/policyd-override', pending_id='',
+                                timestamp='0001-01-01T00:00:00Z', username='', name='policyd-override',
+                                origin='upload', type='file', path='policyd-override.zip',
+                                description='The policy.doverrides file',
+                                revision=0, fingerprint='OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb',
+                                size=0)}
+        assert not utils.should_upgrade_resource(res, existing)
+
+    def test_should_upgrade_resource_yes_new_revision(self):
+        # fields are trimmed for readability
+        res = {'created-at': '2019-10-24T20:45:19.201000',
+               'description': 'The policy.d overrides file',
+               'download': {'hash-sha-256': 'e3b0c4', 'hash-sha-384': '38b060a751ac914898b95b',
+                            'hash-sha-512': 'cf83e1357eef1a538327af927da3e',
+                            'hash-sha3-384': '0c63a75b1bbed1e058d5f004',
+                            'size': 0,
+                            'url': 'https://api.charmhub.io/api/v1/resMGU0L516cGTTwNam.policyd-override_0'},
+               'filename': 'policyd-override.zip', 'name': 'policyd-override', 'revision': 1,
+               'type': 'file'}
+
+        existing = {
+            'policyd-override':
+                client.Resource(charmresource=None,
+                                application='keystone', id_='keystone/policyd-override', pending_id='',
+                                timestamp='0001-01-01T00:00:00Z', username='', name='policyd-override',
+                                origin='store', type='file', path='policyd-override.zip',
+                                description='The policy.doverrides file',
+                                revision=0, fingerprint='OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb',
+                                size=0)}
+        assert utils.should_upgrade_resource(res, existing)