From 44809ed0a7a4d3002f7dcfc64a9679d68bb97401 Mon Sep 17 00:00:00 2001 From: Ross Webster Date: Wed, 13 Dec 2023 14:49:17 +1000 Subject: [PATCH 01/22] [QOLSVC-4214] Update package resource visibility on new file upload --- ckanext/s3filestore/uploader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckanext/s3filestore/uploader.py b/ckanext/s3filestore/uploader.py index ecea046..c04ac51 100644 --- a/ckanext/s3filestore/uploader.py +++ b/ckanext/s3filestore/uploader.py @@ -660,7 +660,7 @@ def upload(self, id, max_size=10): filepath = self.get_path(id, self.filename) self.upload_to_key(filepath, self.upload_file, acl=self._get_target_acl(id), extra_metadata=self._get_resource_metadata()) - self.update_visibility(id) + self.update_visibility(id) # The resource form only sets self.clear (via the input clear_upload) # to True when an uploaded file is not replaced by another uploaded From e68fdeef7a743f76de51e2d9c8731d2313a10a44 Mon Sep 17 00:00:00 2001 From: Ross Webster Date: Wed, 13 Dec 2023 14:55:52 +1000 Subject: [PATCH 02/22] [QOLSVC-4214] Fix lint --- ckanext/s3filestore/tests/test_uploader.py | 2 +- ckanext/s3filestore/views/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ckanext/s3filestore/tests/test_uploader.py b/ckanext/s3filestore/tests/test_uploader.py index 4a498f6..50dd957 100644 --- a/ckanext/s3filestore/tests/test_uploader.py +++ b/ckanext/s3filestore/tests/test_uploader.py @@ -494,7 +494,7 @@ def test_ignoring_non_uploads(self): assert_equal(resource['url'], 'https://example.com') assert_is_none(getattr(uploader, 'filename', None)) assert_is_none(getattr(uploader, 'filesize', None)) - with mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.update_visibility') as mock_update_visibility,\ + with mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.update_visibility') as mock_update_visibility, \ mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.upload_to_key') as mock_upload_to_key: uploader.upload(resource['id']) mock_upload_to_key.assert_not_called() diff --git a/ckanext/s3filestore/views/__init__.py b/ckanext/s3filestore/views/__init__.py index ac893f6..e9e9860 100644 --- a/ckanext/s3filestore/views/__init__.py +++ b/ckanext/s3filestore/views/__init__.py @@ -11,7 +11,7 @@ from ckanext.s3filestore import uploader from ckan.lib.uploader import ResourceUpload as DefaultResourceUpload -from ckan.plugins.toolkit import abort, _, g, get_action,\ +from ckan.plugins.toolkit import abort, _, g, get_action, \ NotAuthorized, ObjectNotFound, redirect_to from ckanext.s3filestore.uploader import S3Uploader, BaseS3Uploader From dd0b95df473440bf3b974e90be2bac71a065ac21 Mon Sep 17 00:00:00 2001 From: Ross Webster Date: Wed, 13 Dec 2023 15:48:20 +1000 Subject: [PATCH 03/22] [QOLSVC-4214] Fix 2.10 compatibility for after_update rename --- ckanext/s3filestore/plugin.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ckanext/s3filestore/plugin.py b/ckanext/s3filestore/plugin.py index d6e09ac..b36e835 100644 --- a/ckanext/s3filestore/plugin.py +++ b/ckanext/s3filestore/plugin.py @@ -81,7 +81,12 @@ def get_uploader(self, upload_to, old_filename=None): # IPackageController + # CKAN < 2.10 def after_update(self, context, pkg_dict): + return self.after_dataset_update(context, pkg_dict) + + # CKAN >= 2.10 + def after_dataset_update(self, context, pkg_dict): ''' Update the access of each S3 object to match the package. ''' pkg_id = pkg_dict['id'] From f5ce20536a8fe05809e776bab225290385f07de8 Mon Sep 17 00:00:00 2001 From: Ross Webster Date: Wed, 13 Dec 2023 15:48:54 +1000 Subject: [PATCH 04/22] [QOLSVC-4214] Adjust test logic for update_visibility changes --- ckanext/s3filestore/tests/test_uploader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckanext/s3filestore/tests/test_uploader.py b/ckanext/s3filestore/tests/test_uploader.py index 50dd957..3fc155d 100644 --- a/ckanext/s3filestore/tests/test_uploader.py +++ b/ckanext/s3filestore/tests/test_uploader.py @@ -498,7 +498,7 @@ def test_ignoring_non_uploads(self): mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.upload_to_key') as mock_upload_to_key: uploader.upload(resource['id']) mock_upload_to_key.assert_not_called() - mock_update_visibility.assert_called_once_with(resource['id']) + mock_update_visibility.assert_not_called() def test_detect_office_document_type(self): dataset = self._test_dataset() From e14b41d5337b962a1dbad5fa45f212f7877f9e1f Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Wed, 21 Feb 2024 08:49:15 +1000 Subject: [PATCH 05/22] add source installation procedure to the README, GitHub #112 --- README.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 380a68b..8a741cf 100644 --- a/README.rst +++ b/README.rst @@ -37,10 +37,17 @@ To install ckanext-s3filestore: . /usr/lib/ckan/default/bin/activate -2. Install the ckanext-s3filestore Python package into your virtual environment:: +2. Install the ckanext-s3filestore Python package into your virtual environment: + + 2.1 From PyPI:: pip install ckanext-s3filestore + 2.2 Alternatively, install from source:: + + pip install -e https://github.com/qld-gov-au/ckanext-s3filestore.git#egg=ckanext-s3filestore + pip install -r /usr/lib/ckan/default/src/ckanext-s3filestore/requirements.txt + 3. Add ``s3filestore`` to the ``ckan.plugins`` setting in your CKAN config file (by default the config file is located at ``/etc/ckan/default/production.ini``). From 8da3e3c7ae4df26d326b9f82de1260827fb59af5 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Wed, 20 Mar 2024 09:30:28 +1000 Subject: [PATCH 06/22] [QOLSVC-4988] don't log data dict that could contain passwords --- ckanext/s3filestore/uploader.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ckanext/s3filestore/uploader.py b/ckanext/s3filestore/uploader.py index c04ac51..e816d35 100644 --- a/ckanext/s3filestore/uploader.py +++ b/ckanext/s3filestore/uploader.py @@ -371,8 +371,8 @@ def update_data_dict(self, data_dict, url_field, file_field, clear_field): pass data_dict[url_field] = self.filename self.upload_file = _get_underlying_file(self.upload_field_storage) - log.debug("ckanext.s3filestore.uploader: is allowed upload type: filename: %s, upload_file: %s, data_dict: %s", - self.filename, self.upload_file, data_dict) + log.debug("ckanext.s3filestore.uploader: is allowed upload type: filename: %s, upload_file: %s", + self.filename, self.upload_file) # keep the file if there has been no change elif self.old_filename and not self.old_filename.startswith('http'): if not self.clear: @@ -381,8 +381,8 @@ def update_data_dict(self, data_dict, url_field, file_field, clear_field): data_dict[url_field] = '' else: log.debug( - "ckanext.s3filestore.uploader: is not allowed upload type: upload_field_storage: %s, data_dict: %s", - self.upload_field_storage, data_dict) + "ckanext.s3filestore.uploader: is not allowed upload type: upload_field_storage: %s", + self.upload_field_storage) def upload(self, max_size=2): log.debug( @@ -647,6 +647,7 @@ def update_visibility(self, id, target_acl=None): log.debug("Updating ACL for object %s to %s", upload_key, acl) client.put_object_acl( Bucket=self.bucket_name, Key=upload_key, ACL=acl) + # Drop the cached URL since it will likely need to change self.redis.delete(upload_key) self.redis.put(upload_key + VISIBILITY_CACHE_PATH, acl, expiry=self.acl_cache_window) self.redis.put(current_key + VISIBILITY_CACHE_PATH + '/all', target_acl, expiry=self.acl_cache_window) From d112ad369298638e8f2a09d6f40031c85d9548ec Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Thu, 21 Mar 2024 09:27:05 +1000 Subject: [PATCH 07/22] [QOLSVC-4988] drop another log that could contain passwords --- ckanext/s3filestore/uploader.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ckanext/s3filestore/uploader.py b/ckanext/s3filestore/uploader.py index e816d35..3bcfd27 100644 --- a/ckanext/s3filestore/uploader.py +++ b/ckanext/s3filestore/uploader.py @@ -332,8 +332,6 @@ def get_storage_path(cls, upload_to): return os.path.join(path, 'storage', 'uploads', upload_to) def update_data_dict(self, data_dict, url_field, file_field, clear_field): - log.debug("ckanext.s3filestore.uploader: update_data_dict: %s, url %s, file %s, clear %s", - data_dict, url_field, file_field, clear_field) '''Manipulate data from the data_dict. This needs to be called before it reaches any validators. From d63fcc46574d74d8a198884b7f813a77c1d60b2d Mon Sep 17 00:00:00 2001 From: snyk-bot Date: Sat, 22 Jun 2024 05:40:59 +0000 Subject: [PATCH 08/22] fix: dev-requirements.txt to reduce vulnerabilities The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-URLLIB3-7267250 --- dev-requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index dfd95ea..0181b1f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -6,4 +6,5 @@ httpretty==0.9.7 parameterized==0.8.1 pytest-cov pytest-ckan -six>=1.13.0 \ No newline at end of file +six>=1.13.0 +urllib3>=2.2.2 # not directly required, pinned by Snyk to avoid a vulnerability \ No newline at end of file From 839cffb833117aeafb6bd13454911c2f0db1f8e9 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 9 Jul 2024 16:58:52 +1000 Subject: [PATCH 09/22] pin urllib3 to the 1.x series instead of 2.x for compatibility with botocore --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 0181b1f..1026aea 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -7,4 +7,4 @@ parameterized==0.8.1 pytest-cov pytest-ckan six>=1.13.0 -urllib3>=2.2.2 # not directly required, pinned by Snyk to avoid a vulnerability \ No newline at end of file +urllib3>=1.26.19 # not directly required, pinned to avoid a vulnerability From 1b8a9a079174e87ac2345dcacca425849bd8d766 Mon Sep 17 00:00:00 2001 From: snyk-bot Date: Sat, 13 Jul 2024 03:13:16 +0000 Subject: [PATCH 10/22] fix: dev-requirements.txt to reduce vulnerabilities The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-ZIPP-7430899 --- dev-requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index dfd95ea..995de4b 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -6,4 +6,5 @@ httpretty==0.9.7 parameterized==0.8.1 pytest-cov pytest-ckan -six>=1.13.0 \ No newline at end of file +six>=1.13.0 +zipp>=3.19.1 # not directly required, pinned by Snyk to avoid a vulnerability \ No newline at end of file From c9688087673226d2faa2897532f9becc327b49a6 Mon Sep 17 00:00:00 2001 From: antuarc Date: Thu, 8 Aug 2024 15:30:21 +1000 Subject: [PATCH 11/22] drop Python 2 testing as it's long past support --- .github/workflows/ci.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db8b0c6..2fef807 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,17 +36,14 @@ jobs: needs: code_quality strategy: matrix: - ckan-version: ['2.10', '2.9', '2.9-py2', '2.8', '2.7'] + ckan-version: ['2.10', '2.9'] ckan-git-org: ['ckan'] include: - - ckan-version: '2.8' - ckan-git-version: 'ckan-2.8.8-qgov.5' - ckan-git-org: 'qld-gov-au' - ckan-version: '2.9' - ckan-git-version: 'ckan-2.9.5-qgov.10' + ckan-git-version: 'ckan-2.9.9-qgov.4' ckan-git-org: 'qld-gov-au' - ckan-version: '2.9' - ckan-git-version: 'qgov-master-2.9.7' + ckan-git-version: 'qgov-master-2.9.9' ckan-git-org: 'qld-gov-au' fail-fast: false From e7e9c63aa0a11c9d7d960eb4f81758d42c899778 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 16:39:01 +1000 Subject: [PATCH 12/22] [QOLDEV-1003] add CKAN 2.11 test run --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2fef807..8b832a5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: needs: code_quality strategy: matrix: - ckan-version: ['2.10', '2.9'] + ckan-version: ['2.11', '2.10', '2.9'] ckan-git-org: ['ckan'] include: - ckan-version: '2.9' From 85ccd7a88dc5555324724395f1426f3fe1bd6274 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 16:41:16 +1000 Subject: [PATCH 13/22] [QOLDEV-1003] update test containers to handle CKAN 2.11 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b832a5..f316fea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,11 +11,11 @@ jobs: code_quality: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 timeout-minutes: 2 - name: Setup Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: '3.x' timeout-minutes: 5 @@ -50,7 +50,7 @@ jobs: name: CKAN ${{ matrix.ckan-version }} ${{ matrix.ckan-git-org }} ${{ matrix.ckan-git-version }} container: - image: openknowledge/ckan-dev:${{ matrix.ckan-version }} + image: ckan/ckan-dev:${{ matrix.ckan-version }} services: postgresql: @@ -83,7 +83,7 @@ jobs: - "5000" solr: - image: ckan/ckan-solr:${{ matrix.ckan-version }} + image: ckan/ckan-solr:${{ matrix.ckan-version }}-solr9 env: CKAN_SQLALCHEMY_URL: postgresql://ckan_default:pass@postgresql/ckan_test From e4b2335a2cf0a602721acc4b34b6fa122ae93f10 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 16:49:24 +1000 Subject: [PATCH 14/22] [QOLDEV-1003] drop references to 'sudo' - Script can be called as root instead of using sudo --- scripts/build.sh | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/scripts/build.sh b/scripts/build.sh index bf13757..5fceb8a 100644 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -4,8 +4,6 @@ # set -ex - - if [ "$CKAN_GIT_ORG" != "ckan" ]; then SRC_DIR=/srv/app/src APP_DIR=/srv/app @@ -13,21 +11,20 @@ if [ "$CKAN_GIT_ORG" != "ckan" ]; then pushd ${APP_DIR} echo "pip install -e git+https://github.com/${CKAN_GIT_ORG}/ckan.git@${CKAN_GIT_VERSION}#egg=ckan" echo "update manually as its already a git pip installed module" - sudo git config --global --add safe.directory "$SRC_DIR/ckan" + git config --global --add safe.directory "$SRC_DIR/ckan" popd pushd ${SRC_DIR}/ckan - sudo git remote set-url origin "https://github.com/${CKAN_GIT_ORG}/ckan.git" - time sudo git fetch origin #could be the tag/branch only if download time is slow - sudo git clean -f - sudo git reset --hard - sudo find . -name '*.pyc' -delete - sudo git checkout "${CKAN_GIT_VERSION}" + git remote set-url origin "https://github.com/${CKAN_GIT_ORG}/ckan.git" + time git fetch origin #could be the tag/branch only if download time is slow + git clean -f + git reset --hard + find . -name '*.pyc' -delete + git checkout "${CKAN_GIT_VERSION}" pip install --no-binary :all: -r requirements.txt popd - fi pip install -r requirements.txt @@ -35,4 +32,3 @@ pip install -r dev-requirements.txt pip install -e . # Replace default path to CKAN core config file with the one on the container sed -i -e "s|use = config:.*|use = config:${SRC_DIR}/ckan/test-core.ini|" test.ini - From e8f7dc09df42005175ec0b54342e5d174d8c1b79 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 16:53:33 +1000 Subject: [PATCH 15/22] [QOLDEV-1003] pin setuptools when needed for CKAN 2.9 --- scripts/build.sh | 3 +++ test.ini | 2 ++ 2 files changed, 5 insertions(+) diff --git a/scripts/build.sh b/scripts/build.sh index 5fceb8a..34699e4 100644 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -4,6 +4,9 @@ # set -ex +if [ "$CKAN_VERSION" = "2.9" ]; then + pip install "setuptools>=44.1.0,<71" +fi if [ "$CKAN_GIT_ORG" != "ckan" ]; then SRC_DIR=/srv/app/src APP_DIR=/srv/app diff --git a/test.ini b/test.ini index 79ec5ee..1ee1102 100644 --- a/test.ini +++ b/test.ini @@ -13,6 +13,8 @@ port = 5000 [app:main] use = config:../ckan/test-core.ini +SECRET_KEY=foo + # Insert any custom config settings to be used when running your extension's # tests here. From 066d2250f85c04435f0d27164bea1ae10c0b891f Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 17:06:54 +1000 Subject: [PATCH 16/22] [QOLDEV-1003] update test dependencies to handle CKAN 2.11 --- dev-requirements.txt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 3a116d2..99bdad6 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,9 +1,8 @@ -moto==2.0.1 +moto==5.0.20 ckanapi==4.3 -#1.0.2 is python3 only -httpretty==0.9.7 -parameterized==0.8.1 +httpretty==1.1.4 +parameterized==0.9.0 pytest-cov pytest-ckan six>=1.13.0 From 6d903049ac7ab0ad01d1ab58d2b1940a74415721 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Mon, 11 Nov 2024 17:28:23 +1000 Subject: [PATCH 17/22] [QOLDEV-1003] replace Nosetests syntax with pytest --- README.rst | 2 +- ckanext/s3filestore/tests/__init__.py | 6 - ckanext/s3filestore/tests/test_controller.py | 21 ++-- ckanext/s3filestore/tests/test_plugin.py | 7 +- ckanext/s3filestore/tests/test_uploader.py | 113 +++++++++---------- 5 files changed, 71 insertions(+), 78 deletions(-) diff --git a/README.rst b/README.rst index 8a741cf..b5fdb1f 100644 --- a/README.rst +++ b/README.rst @@ -215,7 +215,7 @@ apt-get install sudo systemd postgresql-10 git python python-pip export PGVERSION=10 && export CKAN_BRANCH=qgov-master && export CKAN_GIT_REPO=qld-gov-au/ckan cd /build bash bin/travis-build.bash -nosetests --ckan --with-pylons=subdir/test.ini --with-coverage --cover-package=ckanext.s3filestore --cover-inclusive --cover-erase --cover-tests +pytest --ckan-ini=test.ini --cov=ckanext.s3filestore --------------------------------------- Registering ckanext-s3filestore on PyPI diff --git a/ckanext/s3filestore/tests/__init__.py b/ckanext/s3filestore/tests/__init__.py index 872da3b..8b9e16c 100644 --- a/ckanext/s3filestore/tests/__init__.py +++ b/ckanext/s3filestore/tests/__init__.py @@ -1,7 +1,5 @@ # encoding: utf-8 -from ckan.tests import helpers - def _get_status_code(response): """ Get the status code from a HTTP response. @@ -20,7 +18,3 @@ def _get_response_body(response): return response.text else: return response.body - - -def teardown_function(self=None): - helpers.reset_db() diff --git a/ckanext/s3filestore/tests/test_controller.py b/ckanext/s3filestore/tests/test_controller.py index 6475d79..de2cefe 100644 --- a/ckanext/s3filestore/tests/test_controller.py +++ b/ckanext/s3filestore/tests/test_controller.py @@ -6,7 +6,7 @@ import requests import six -from nose.tools import (with_setup) +import pytest from werkzeug.datastructures import FileStorage as FlaskFileStorage @@ -18,19 +18,11 @@ from ckanext.s3filestore import uploader -from . import _get_status_code, _get_response_body, teardown_function +from . import _get_status_code, _get_response_body log = logging.getLogger(__name__) -def setup_function(self): - self.sysadmin = factories.Sysadmin(apikey="my-test-key") - - assert config.get('ckanext.s3filestore.signature_version') == 's3v4' - self.bucket_name = config.get(u'ckanext.s3filestore.aws_bucket_name') - uploader.BaseS3Uploader().get_s3_bucket(self.bucket_name) - - def _test_org(): try: return helpers.call_action('organization_show', id='test-org') @@ -47,9 +39,16 @@ def _test_org(): image_upload=FlaskFileStorage(six.BytesIO(b"\0\0\0"), u"image.png")) -@with_setup(setup_function, teardown_function) +@pytest.mark.usefixtures("clean_db", "with_plugins") class TestS3Controller(object): + def setup_method(self, test_method): + self.sysadmin = factories.Sysadmin(apikey="my-test-key") + + assert config.get('ckanext.s3filestore.signature_version') == 's3v4' + self.bucket_name = config.get(u'ckanext.s3filestore.aws_bucket_name') + uploader.BaseS3Uploader().get_s3_bucket(self.bucket_name) + def _upload_resource(self): dataset = factories.Dataset(name="my-dataset") diff --git a/ckanext/s3filestore/tests/test_plugin.py b/ckanext/s3filestore/tests/test_plugin.py index 6c826ed..e3f72fc 100644 --- a/ckanext/s3filestore/tests/test_plugin.py +++ b/ckanext/s3filestore/tests/test_plugin.py @@ -1,6 +1,9 @@ # encoding: utf-8 -import mock +try: + from unittest import mock +except ImportError: + import mock from parameterized import parameterized import ckantoolkit as toolkit @@ -11,7 +14,7 @@ class TestS3Plugin(): - def setup(self): + def setup_method(self, test_method): self.plugin = S3FileStorePlugin() def test_update_config(self): diff --git a/ckanext/s3filestore/tests/test_uploader.py b/ckanext/s3filestore/tests/test_uploader.py index 3fc155d..7c9cc1c 100644 --- a/ckanext/s3filestore/tests/test_uploader.py +++ b/ckanext/s3filestore/tests/test_uploader.py @@ -5,14 +5,12 @@ import os import six -import mock -from nose.tools import (assert_equal, - assert_true, - assert_false, - assert_in, - assert_is_none, - assert_raises, - with_setup) +try: + from unittest import mock +except ImportError: + import mock + +import pytest from botocore.exceptions import ClientError @@ -45,10 +43,6 @@ def _setup_function(self): uploader.get_s3_bucket(self.bucket_name) -def _resource_setup_function(self): - _setup_function(self) - - def _get_object_key(resource): ''' Determine the S3 object key for the specified resource. ''' @@ -58,34 +52,36 @@ def _get_object_key(resource): def _assert_public(resource, url, uploader): - assert_false(_is_presigned_url(url), "Expected {} [{}] to use public URL but was {}".format( - resource, uploader.get_path(resource['id']), url)) + assert not _is_presigned_url(url), "Expected {} [{}] to use public URL but was {}".format( + resource, uploader.get_path(resource['id']), url) def _assert_private(resource, url, uploader): - assert_true(_is_presigned_url(url), "Expected {} [{}] to use private URL but was {}".format( - resource, uploader.get_path(resource['id']), url)) + assert _is_presigned_url(url), "Expected {} [{}] to use private URL but was {}".format( + resource, uploader.get_path(resource['id']), url) -@with_setup(_setup_function) class TestS3Uploader(): + def setup_method(self, test_method): + _setup_function(self) + def test_get_bucket(self): '''S3Uploader retrieves bucket as expected''' uploader = S3Uploader('') - assert_true(uploader.get_s3_bucket(self.bucket_name)) + assert uploader.get_s3_bucket(self.bucket_name) def test_clean_dict(self): '''S3Uploader retrieves bucket as expected''' uploader = S3Uploader('') date_dict = {'key': datetime.datetime(1970, 1, 2, 3, 4, 5, 6)} clean_dict = uploader.as_clean_dict(date_dict) - assert_equal(clean_dict['key'], '1970-01-02T03:04:05.000006') + assert clean_dict['key'] == '1970-01-02T03:04:05.000006' def test_uploader_storage_path(self): '''S3Uploader get_storage_path returns as expected''' returned_path = S3Uploader.get_storage_path('myfiles') - assert_equal(returned_path, 'my-path/storage/uploads/myfiles') + assert returned_path == 'my-path/storage/uploads/myfiles' def test_group_image_upload(self): '''Test a group image file upload''' @@ -116,9 +112,9 @@ def test_group_image_upload(self): # attempt redirect to linked url image_file_url = '/uploads/group/2001-01-29-000000{0}'.format(file_name) status_code, location = self._get_expecting_redirect(self.app, image_file_url) - assert_equal(location.split('?')[0], - '{0}/my-bucket/my-path/storage/uploads/group/2001-01-29-000000{1}' - .format(self.endpoint_url, file_name)) + assert location.split('?')[0] == \ + '{0}/my-bucket/my-path/storage/uploads/group/2001-01-29-000000{1}'.format( + self.endpoint_url, file_name) def test_group_image_upload_then_clear(self): '''Test that clearing an upload removes the S3 key''' @@ -153,18 +149,18 @@ def test_group_image_upload_then_clear(self): try: self.s3.head_object(Bucket=self.bucket_name, Key=key) # broken by https://github.com/ckan/ckan/commit/48afb9da4d - # assert_false(True, "file '{}' should not exist".format(key)) + # assert False, "file '{}' should not exist".format(key) except ClientError: # passed - assert_true(True, "passed") + assert True, "passed" if toolkit.check_ckan_version('2.9'): def _get_expecting_redirect(self, app, url): response = app.get(url, follow_redirects=False) status_code = _get_status_code(response) - assert_in(status_code, [301, 302], - "%s resulted in %s instead of a redirect" % (url, status_code)) + assert status_code in [301, 302], \ + "%s resulted in %s instead of a redirect" % (url, status_code) return status_code, response.location else: @@ -172,14 +168,16 @@ def _get_expecting_redirect(self, app, url): def _get_expecting_redirect(self, app, url): response = app.get(url) status_code = _get_status_code(response) - assert_in(status_code, [301, 302], - "%s resulted in %s instead of a redirect" % (url, status_code)) + assert status_code in [301, 302], \ + "%s resulted in %s instead of a redirect" % (url, status_code) return status_code, response.headers['Location'] -@with_setup(_resource_setup_function) class TestS3ResourceUploader(): + def setup_method(self, test_method): + _setup_function(self) + def _test_dataset(self, private=False, title='Test Dataset', author='test'): ''' Creates a test dataset. ''' @@ -207,7 +205,7 @@ def test_resource_upload(self): '''Test a basic resource file upload''' file_path = os.path.join(os.path.dirname(__file__), 'data.csv') resource = self._upload_test_resource() - assert_equal(resource['mimetype'], 'text/csv') + assert resource['mimetype'] == 'text/csv' key = _get_object_key(resource) @@ -218,7 +216,7 @@ def test_resource_upload(self): # test the file contains what's expected obj = self.s3.get_object(Bucket=self.bucket_name, Key=key) data = obj['Body'].read() - assert_equal(data, io.open(file_path, 'rb').read()) + assert data == io.open(file_path, 'rb').read() def test_package_update(self): ''' Test a typical package_update API call. @@ -248,13 +246,12 @@ def test_uploader_get_path(self): uploader = S3ResourceUploader(resource) returned_path = uploader.get_path(resource['id'], 'myfile.txt') - assert_equal(returned_path, - 'my-path/resources/{0}/myfile.txt'.format(resource['id'])) + assert returned_path == 'my-path/resources/{0}/myfile.txt'.format(resource['id']) def test_is_presigned_url(self): ''' Tests that presigned URLs are correctly recognised.''' - assert_true(_is_presigned_url('https://example.s3.amazonaws.com/resources/foo?AWSAccessKeyId=SomeKey&Expires=9999999999Signature=hb7%2F%2Bz1H%2B8wdEy0pCsX7bZG%2BuPU%3D')) - assert_false(_is_presigned_url('https://example.s3.amazonaws.com/resources/foo')) + assert _is_presigned_url('https://example.s3.amazonaws.com/resources/foo?AWSAccessKeyId=SomeKey&Expires=9999999999Signature=hb7%2F%2Bz1H%2B8wdEy0pCsX7bZG%2BuPU%3D') + assert not _is_presigned_url('https://example.s3.amazonaws.com/resources/foo') @helpers.change_config('ckanext.s3filestore.acl', 'auto') def test_resource_url_unsigned_for_public_dataset(self): @@ -267,7 +264,7 @@ def test_resource_url_unsigned_for_public_dataset(self): url = uploader.get_signed_url_to_key(key) _assert_public(resource, url, uploader) - assert_in('ETag=', url) + assert 'ETag=' in url @helpers.change_config('ckanext.s3filestore.acl', 'auto') def test_resource_url_signed_for_private_dataset(self): @@ -294,7 +291,7 @@ def test_making_dataset_private_updates_object_visibility(self): url = uploader.get_signed_url_to_key(key) _assert_public(resource, url, uploader) - assert_in('ETag=', url) + assert 'ETag=' in url helpers.call_action('package_patch', context={'user': self.sysadmin['name']}, @@ -324,7 +321,7 @@ def test_making_dataset_public_updates_object_visibility(self): url = uploader.get_signed_url_to_key(key) _assert_public(resource, url, uploader) - assert_in('ETag=', url) + assert 'ETag=' in url @helpers.change_config('ckanext.s3filestore.acl', 'auto') def test_non_current_objects_are_private(self): @@ -344,11 +341,11 @@ def test_non_current_objects_are_private(self): key = uploader.get_path(resource['id']) url = uploader.get_signed_url_to_key(key) - assert_false(_is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url)) + assert not _is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url) key = uploader.get_path(resource['id'], 'data.csv') url = uploader.get_signed_url_to_key(key) - assert_true(_is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url)) + assert _is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url) @helpers.change_config('ckanext.s3filestore.acl', 'auto') @helpers.change_config('ckanext.s3filestore.non_current_acl', 'public-read') @@ -369,11 +366,11 @@ def test_non_current_objects_match_acl(self): key = uploader.get_path(resource['id']) url = uploader.get_signed_url_to_key(key) - assert_false(_is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url)) + assert not _is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url) key = uploader.get_path(resource['id'], 'data.csv') url = uploader.get_signed_url_to_key(key) - assert_false(_is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url)) + assert not _is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url) @helpers.change_config('ckanext.s3filestore.acl', 'auto') @helpers.change_config('ckanext.s3filestore.non_current_acl', 'auto') @@ -394,11 +391,11 @@ def test_non_current_objects_match_auto_acl(self): key = uploader.get_path(resource['id']) url = uploader.get_signed_url_to_key(key) - assert_false(_is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url)) + assert not _is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url) key = uploader.get_path(resource['id'], 'data.csv') url = uploader.get_signed_url_to_key(key) - assert_false(_is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url)) + assert not _is_presigned_url(url), "Expected [{}] to use public URL but was {}".format(key, url) helpers.call_action( 'package_patch', @@ -406,7 +403,7 @@ def test_non_current_objects_match_auto_acl(self): private=True ) url = uploader.get_signed_url_to_key(key) - assert_true(_is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url)) + assert _is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url) @helpers.change_config('ckanext.s3filestore.acl', 'auto') @helpers.change_config('ckanext.s3filestore.delete_non_current_days', '0') @@ -429,7 +426,7 @@ def test_delete_non_current_objects_after_expiry(self): assert uploader.get_signed_url_to_key(key) is not None key = uploader.get_path(resource['id'], 'data.csv') - with assert_raises(toolkit.ObjectNotFound): + with pytest.raises(toolkit.ObjectNotFound): assert uploader.get_signed_url_to_key(key) is not None @helpers.change_config('ckanext.s3filestore.acl', 'auto') @@ -454,7 +451,7 @@ def test_do_not_delete_non_current_objects_before_expiry(self): key = uploader.get_path(resource['id'], 'data.csv') url = uploader.get_signed_url_to_key(key) - assert_true(_is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url)) + assert _is_presigned_url(url), "Expected [{}] to use private URL but was {}".format(key, url) def test_assembling_object_metadata_headers(self): ''' Tests that text fields from the package are passed to S3. @@ -464,8 +461,8 @@ def test_assembling_object_metadata_headers(self): uploader = S3ResourceUploader(resource) object_metadata = uploader._get_resource_metadata() - assert_equal(object_metadata['package_id'], dataset['id']) - assert_false('notes' in object_metadata['package_id']) + assert object_metadata['package_id'] == dataset['id'] + assert 'notes' not in object_metadata['package_id'] def test_encoding_object_metadata_headers(self): ''' Tests that text fields from the package are passed to S3. @@ -475,8 +472,8 @@ def test_encoding_object_metadata_headers(self): uploader = S3ResourceUploader(resource) object_metadata = uploader._get_resource_metadata() - assert_equal(object_metadata['package_title'], 'Test Dataset—with em dash') - assert_equal(object_metadata['package_author'], '擬製 暗影') + assert object_metadata['package_title'] == 'Test Dataset—with em dash' + assert object_metadata['package_author'] == '擬製 暗影' def test_ignoring_non_uploads(self): ''' Tests that resources not containing an upload are ignored. @@ -491,9 +488,9 @@ def test_ignoring_non_uploads(self): ] for resource in resources: uploader = S3ResourceUploader(resource) - assert_equal(resource['url'], 'https://example.com') - assert_is_none(getattr(uploader, 'filename', None)) - assert_is_none(getattr(uploader, 'filesize', None)) + assert resource['url'] == 'https://example.com' + assert getattr(uploader, 'filename', None) is None + assert getattr(uploader, 'filesize', None) is None with mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.update_visibility') as mock_update_visibility, \ mock.patch('ckanext.s3filestore.uploader.S3ResourceUploader.upload_to_key') as mock_upload_to_key: uploader.upload(resource['id']) @@ -503,7 +500,7 @@ def test_ignoring_non_uploads(self): def test_detect_office_document_type(self): dataset = self._test_dataset() resource = self._upload_test_resource(dataset, 'example.docx') - assert_equal(resource['mimetype'], 'application/vnd.openxmlformats-officedocument.wordprocessingml.document') + assert resource['mimetype'] == 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' if toolkit.check_ckan_version(max_version='2.8.99'): @@ -531,7 +528,7 @@ def test_resource_upload_then_clear(self): # key shouldn't exist try: self.s3.head_object(Bucket=self.bucket_name, Key=key) - assert_false(True, "file should not exist") + assert False, "file should not exist" except ClientError: # passed - assert_true(True, "passed") + assert True, "passed" From 68b7cac6df7ee64fa71538bcb951f98c5e35321f Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 12 Nov 2024 09:05:03 +1000 Subject: [PATCH 18/22] [QOLDEV-1003] add testing of CKAN upstream master - Run tests on latest CKAN master branch for visibility, but don't require success --- .github/workflows/ci.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f316fea..3cb0689 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,7 @@ jobs: strategy: matrix: ckan-version: ['2.11', '2.10', '2.9'] + experimental: [false] ckan-git-org: ['ckan'] include: - ckan-version: '2.9' @@ -45,6 +46,10 @@ jobs: - ckan-version: '2.9' ckan-git-version: 'qgov-master-2.9.9' ckan-git-org: 'qld-gov-au' + - ckan-version: 'master' + ckan-git-org: 'ckan' + ckan-git-version: 'master' + experimental: true #master is unstable, good to know if we are compatible or not fail-fast: false @@ -101,17 +106,18 @@ jobs: timeout-minutes: 2 - name: Install requirements + continue-on-error: ${{ matrix.experimental }} run: | chmod u+x ./scripts/* ./scripts/build.sh timeout-minutes: 15 - name: Setup CKAN - run: | - ./scripts/init.sh + continue-on-error: ${{ matrix.experimental }} + run: ./scripts/init.sh timeout-minutes: 15 - name: Run tests - run: | - ./scripts/test.sh + continue-on-error: ${{ matrix.experimental }} + run: ./scripts/test.sh timeout-minutes: 30 From e97d6810f48cfd43e30cf82f15939d8c4890d9e5 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 12 Nov 2024 09:38:08 +1000 Subject: [PATCH 19/22] [QOLDEV-1003] increase pytest verbosity - We want to see entire dicts on failure --- scripts/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test.sh b/scripts/test.sh index 78f6d45..32591be 100644 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,3 +1,3 @@ #!/usr/bin/env bash -pytest --ckan-ini=test.ini --cov=ckanext.s3filestore \ No newline at end of file +pytest -vv --ckan-ini=test.ini --cov=ckanext.s3filestore From efb34a1afa2a69ac13cb7a2df78358d460a76225 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 12 Nov 2024 16:47:14 +1000 Subject: [PATCH 20/22] [QOLDEV-1003] generate JUnit-style test results --- .github/workflows/ci.yml | 6 ++++++ scripts/test.sh | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3cb0689..518b170 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,3 +121,9 @@ jobs: continue-on-error: ${{ matrix.experimental }} run: ./scripts/test.sh timeout-minutes: 30 + + - name: Test Summary + uses: test-summary/action@v2 + with: + paths: "/tmp/artifacts/junit/results.xml" + if: always() diff --git a/scripts/test.sh b/scripts/test.sh index 32591be..7618047 100644 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,3 +1,3 @@ #!/usr/bin/env bash -pytest -vv --ckan-ini=test.ini --cov=ckanext.s3filestore +pytest -vv --ckan-ini=test.ini --cov=ckanext.s3filestore --junit-xml=/tmp/artifacts/junit/results.xml From 401616dced41584e6a40425f5abac4f8e2161362 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 12 Nov 2024 17:04:25 +1000 Subject: [PATCH 21/22] [QOLDEV-1003] gracefully handle resources without IDs - Assume that resources with no ID are new and therefore don't need a visibility update, since they would have been created with the matching visibility. --- ckanext/s3filestore/plugin.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ckanext/s3filestore/plugin.py b/ckanext/s3filestore/plugin.py index b36e835..ca5c395 100644 --- a/ckanext/s3filestore/plugin.py +++ b/ckanext/s3filestore/plugin.py @@ -123,6 +123,9 @@ def after_update_resource_list_update(self, visibility_level, pkg_id, pkg_dict): LOG.debug("after_update_resource_list_update: Package %s has been updated, notifying resources", pkg_id) for resource in pkg_dict['resources']: + if 'id' not in resource: + # skip new resources as they would already have correct visibility + continue uploader = get_resource_uploader(resource) if hasattr(uploader, 'update_visibility'): uploader.update_visibility( From f5a2078d37cdc2add02fa1f09d85a6b4506be809 Mon Sep 17 00:00:00 2001 From: ThrawnCA Date: Tue, 12 Nov 2024 17:08:54 +1000 Subject: [PATCH 22/22] [QOLDEV-1003] update checkout action to use supported Node --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 518b170..cb3afd0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -102,7 +102,7 @@ jobs: steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 timeout-minutes: 2 - name: Install requirements