From 98c2bb704524c5f2700d5954c070af68b1c014be Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Tue, 23 Jul 2024 23:45:44 -0400 Subject: [PATCH 1/3] [#5713] test fixes # Conflicts: # ckan/logic/action/update.py # ckanext/example_iuploader/test/test_plugin.py ### RESOLVED. --- ckan/logic/action/update.py | 25 +++++++++++++++++-- ckan/model/modification.py | 2 ++ ckanext/datapusher/plugin.py | 2 +- ckanext/datapusher/tests/test_action.py | 1 - .../tests/test_example_iresourcecontroller.py | 2 +- ckanext/example_iuploader/test/test_plugin.py | 2 +- 6 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 88bf5ae636c..75e0fc5615b 100644 --- a/ckan/logic/action/update.py +++ b/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) @@ -406,6 +406,16 @@ def package_update( pkg, change = model_save.package_dict_save( data, context, include_plugin_data, copy_resources) + log.info(' ') + log.info('DEBUGGING::package_update::post package_dict_save') + log.info(' ') + log.info(change) + log.info(' ') + log.info(resource_uploads) + log.info(' ') + log.info(changed_resources) + log.info(' ') + if change: if not data.get('metadata_modified'): pkg.metadata_modified = datetime.datetime.utcnow() @@ -419,15 +429,26 @@ def package_update( # Needed to let extensions know the new resources ids model.Session.flush() if changed_resources: + log.info(' ') + log.info('DEBUGGING::package_update::change -> changed_resources') + log.info(' ') resiter = iter(changed_resources) uploaditer = iter(resource_uploads) for i in range(len(pkg.resources)): + log.info(' ') + log.info('DEBUGGING::package_update::STEP 1::loop') + log.info(' ') if i in copy_resources: + log.info(' ') + log.info('DEBUGGING::package_update::STEP 2::no actual change, continue') + log.info(' ') continue resource = next(resiter) upload = next(uploaditer) resource['id'] = pkg.resources[i].id - + log.info(' ') + log.info('DEBUGGING::package_update::STEP 3::uploading resource by id %s' % resource['id']) + log.info(' ') upload.upload(resource['id'], uploader.get_max_resource_size()) for item in plugins.PluginImplementations(plugins.IPackageController): diff --git a/ckan/model/modification.py b/ckan/model/modification.py index 3e89d346173..9fdf001d109 100644 --- a/ckan/model/modification.py +++ b/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: diff --git a/ckanext/datapusher/plugin.py b/ckanext/datapusher/plugin.py index c7cc476f2b7..bd6c83623c4 100644 --- a/ckanext/datapusher/plugin.py +++ b/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) diff --git a/ckanext/datapusher/tests/test_action.py b/ckanext/datapusher/tests/test_action.py index 0ab187c2168..2280dba160a 100644 --- a/ckanext/datapusher/tests/test_action.py +++ b/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() diff --git a/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py b/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py index 0b9f7240d0f..0f8abaf5bd8 100644 --- a/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py +++ b/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 diff --git a/ckanext/example_iuploader/test/test_plugin.py b/ckanext/example_iuploader/test/test_plugin.py index 4d23d8ea503..3022f00fd95 100644 --- a/ckanext/example_iuploader/test/test_plugin.py +++ b/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 From f23b3165015da82afecb7408c80184240eeb2ab6 Mon Sep 17 00:00:00 2001 From: Jesse Vickery Date: Tue, 17 Feb 2026 15:29:51 +0000 Subject: [PATCH 2/3] fix(model): resource order; - Set resource order in the backref `resources_all`. --- ckan/logic/action/update.py | 22 ---------------------- ckan/model/resource.py | 3 +++ 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 75e0fc5615b..327856ca132 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -406,16 +406,6 @@ def package_update( pkg, change = model_save.package_dict_save( data, context, include_plugin_data, copy_resources) - log.info(' ') - log.info('DEBUGGING::package_update::post package_dict_save') - log.info(' ') - log.info(change) - log.info(' ') - log.info(resource_uploads) - log.info(' ') - log.info(changed_resources) - log.info(' ') - if change: if not data.get('metadata_modified'): pkg.metadata_modified = datetime.datetime.utcnow() @@ -429,26 +419,14 @@ def package_update( # Needed to let extensions know the new resources ids model.Session.flush() if changed_resources: - log.info(' ') - log.info('DEBUGGING::package_update::change -> changed_resources') - log.info(' ') resiter = iter(changed_resources) uploaditer = iter(resource_uploads) for i in range(len(pkg.resources)): - log.info(' ') - log.info('DEBUGGING::package_update::STEP 1::loop') - log.info(' ') if i in copy_resources: - log.info(' ') - log.info('DEBUGGING::package_update::STEP 2::no actual change, continue') - log.info(' ') continue resource = next(resiter) upload = next(uploaditer) resource['id'] = pkg.resources[i].id - log.info(' ') - log.info('DEBUGGING::package_update::STEP 3::uploading resource by id %s' % resource['id']) - log.info(' ') upload.upload(resource['id'], uploader.get_max_resource_size()) for item in plugins.PluginImplementations(plugins.IPackageController): diff --git a/ckan/model/resource.py b/ckan/model/resource.py index ec27295ee66..97270bf9cc5 100644 --- a/ckan/model/resource.py +++ b/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' ), ) From 8119eb369fa52f674f4a6d8eef762a53c9266fa4 Mon Sep 17 00:00:00 2001 From: Jesse Vickery Date: Tue, 17 Feb 2026 15:39:24 +0000 Subject: [PATCH 3/3] feat(misc): changelog; - Added change log file. --- changes/222.backport.bugfix | 1 + changes/222.canada.bugfix | 1 + 2 files changed, 2 insertions(+) create mode 100644 changes/222.backport.bugfix create mode 100644 changes/222.canada.bugfix diff --git a/changes/222.backport.bugfix b/changes/222.backport.bugfix new file mode 100644 index 00000000000..a8ef7a99301 --- /dev/null +++ b/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. diff --git a/changes/222.canada.bugfix b/changes/222.canada.bugfix new file mode 100644 index 00000000000..9998e2e1ad9 --- /dev/null +++ b/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`