Fix Flaky Issue with Resource Uploads when Updating

changes/222.backport.bugfix

@@ -0,0 +1 @@
+Backported (d3fff92)[https://github.com/ckan/ckan/commit/d3fff92ebefa1109dfe7cf9c0cad072eaba2bc0b] to fix a flaky issue with resource uploads when updating.

changes/222.canada.bugfix

@@ -0,0 +1 @@
+Added an Order By to the backref `resources_all` so that the Package ORM object accessing its `resources` property will have the resources in the proper ordering of `position`. This fixes a flaky issue when uploading resources during `package_update`

ckan/logic/action/update.py

@@ -346,7 +346,7 @@ def package_update(
         changed_resources = original_package['resources']
 
     resource_uploads = []
-    for resource in data_dict.get('resources', []):
+    for resource in changed_resources or []:
         # file uploads/clearing
         upload = uploader.get_resource_uploader(resource)
 
@@ -427,7 +427,6 @@ def package_update(
                 resource = next(resiter)
                 upload = next(uploaditer)
                 resource['id'] = pkg.resources[i].id
-
                 upload.upload(resource['id'], uploader.get_max_resource_size())
 
         for item in plugins.PluginImplementations(plugins.IPackageController):

ckan/model/modification.py

@@ -67,6 +67,8 @@ def notify_observers(self, session: Any, method: Any):
     def notify(self, entity: Any, operation: Any):
         for observer in plugins.PluginImplementations(
                 plugins.IDomainObjectModification):
+            # FIXME: swallowing exceptions hides issues and makes index_package
+            # difficult to debug
             try:
                 observer.notify(entity, operation)
             except SearchIndexError as search_error:

ckan/model/resource.py

@@ -171,6 +171,9 @@ def related_packages(self) -> list[Package]:
         # formally package_resources_all
         backref=orm.backref('resources_all',
                             collection_class=ordering_list('position'),
+                            # (canada fork only): proper resource position order for package ORM object
+                            # TODO: upstream contrib!!!
+                            order_by=resource_table.c.position,
                             cascade='all, delete'
                             ),
     )

ckanext/datapusher/plugin.py

@@ -75,7 +75,7 @@ def after_resource_create(
 
         self._submit_to_datapusher(resource_dict)
 
-    def after_update(
+    def after_resource_update(
             self, context: Context, resource_dict: dict[str, Any]):
 
         self._submit_to_datapusher(resource_dict)

ckanext/datapusher/tests/test_action.py

@@ -47,7 +47,6 @@ def test_submit(self, monkeypatch):
         func.assert_called()
 
     @pytest.mark.ckan_config("ckan.views.default_views", "")
-    @pytest.mark.flaky(reruns=2)
     def test_submit_when_url_changes(self, monkeypatch):
         dataset = factories.Dataset()
         func = mock.Mock()

ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py

@@ -94,4 +94,4 @@ def test_resource_controller_plugin_show(self):
         assert plugin.counter["after_resource_update"] == 0, plugin.counter
         assert plugin.counter["before_resource_delete"] == 0, plugin.counter
         assert plugin.counter["after_resource_delete"] == 0, plugin.counter
-        assert plugin.counter["before_resource_show"] == 4, plugin.counter
+        assert plugin.counter["before_resource_show"] == 3, plugin.counter

ckanext/example_iuploader/test/test_plugin.py

@@ -59,7 +59,7 @@ def test_resource_download_iuploader_called(
             "save": "go-metadata",
             "upload": ("README.rst", CONTENT)
         })
-    assert mock_get_path.call_count == 1
+    assert mock_get_path.call_count == 2
     assert isinstance(mock_get_path.call_args[0][0], plugin.ResourceUpload)
     pkg = model.Package.by_name(dataset_name)
     assert mock_get_path.call_args[0][1] == pkg.resources[0].id