From f55ac316ea310a4f4f1b85f4e456b8c2d9df3445 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Mon, 10 Mar 2025 15:47:08 +0800 Subject: [PATCH 01/12] Drop pylons support --- ckanext/datapackager/plugin/__init__.py | 36 +++++++++++++++----- ckanext/datapackager/plugin/flask_plugin.py | 15 -------- ckanext/datapackager/plugin/pylons_plugin.py | 27 --------------- 3 files changed, 28 insertions(+), 50 deletions(-) delete mode 100644 ckanext/datapackager/plugin/flask_plugin.py delete mode 100644 ckanext/datapackager/plugin/pylons_plugin.py diff --git a/ckanext/datapackager/plugin/__init__.py b/ckanext/datapackager/plugin/__init__.py index 775d4e7..1902d69 100644 --- a/ckanext/datapackager/plugin/__init__.py +++ b/ckanext/datapackager/plugin/__init__.py @@ -1,21 +1,18 @@ +from flask import Blueprint + import ckan.plugins as plugins import ckan.plugins.toolkit as toolkit +from ckanext.datapackager.controllers import datapackage from ckanext.datapackager.logic.action.create import package_create_from_datapackage from ckanext.datapackager.logic.action.get import package_show_as_datapackage -if toolkit.check_ckan_version(u'2.9'): - from ckanext.datapackager.plugin.flask_plugin import MixinPlugin - ckan_29_or_higher = True -else: - from ckanext.datapackager.plugin.pylons_plugin import MixinPlugin - ckan_29_or_higher = False - -class DataPackagerPlugin(MixinPlugin, plugins.SingletonPlugin): +class DataPackagerPlugin(plugins.SingletonPlugin): '''Plugin that adds importing/exporting datasets as Data Packages. ''' plugins.implements(plugins.IActions) plugins.implements(plugins.IConfigurer) + plugins.implements(plugins.IBlueprint) def update_config(self, config): toolkit.add_template_directory(config, '../templates') @@ -25,3 +22,26 @@ def get_actions(self): 'package_create_from_datapackage': package_create_from_datapackage, 'package_show_as_datapackage': package_show_as_datapackage, } + + def get_blueprint(self): + blueprint = Blueprint("datapackager", __name__) + # As long as the URL for import_datapackage_view and import_datapackage are the same, reverse lookups from import_datapackage will work + blueprint.add_url_rule( + "/import_datapackage", + view_func=datapackage.new, + endpoint="import_datapackage", + methods=["GET"], + ) + blueprint.add_url_rule( + "/import_datapackage", + view_func=datapackage.import_datapackage, + endpoint="import_datapackage_post", + methods=["POST"], + ) + blueprint.add_url_rule( + "/dataset//datapackage.json", + view_func=datapackage.export_datapackage, + endpoint="export_datapackage", + methods=["GET"], + ) + return blueprint diff --git a/ckanext/datapackager/plugin/flask_plugin.py b/ckanext/datapackager/plugin/flask_plugin.py deleted file mode 100644 index b9dc615..0000000 --- a/ckanext/datapackager/plugin/flask_plugin.py +++ /dev/null @@ -1,15 +0,0 @@ -import ckan.plugins as plugins - -from flask import Blueprint -import ckanext.datapackager.controllers.datapackage as datapackage - -class MixinPlugin(plugins.SingletonPlugin): - plugins.implements(plugins.IBlueprint) - - def get_blueprint(self): - blueprint = Blueprint('datapackager', __name__) - # As long as the URL for import_datapackage_view and import_datapackage are the same, reverse lookups from import_datapackage will work - blueprint.add_url_rule("/import_datapackage", view_func=datapackage.new, endpoint='import_datapackage', methods=['GET']) - blueprint.add_url_rule("/import_datapackage", view_func=datapackage.import_datapackage, endpoint='import_datapackage_post', methods=['POST']) - blueprint.add_url_rule("/dataset//datapackage.json", view_func=datapackage.export_datapackage, endpoint='export_datapackage', methods=['GET']) - return blueprint \ No newline at end of file diff --git a/ckanext/datapackager/plugin/pylons_plugin.py b/ckanext/datapackager/plugin/pylons_plugin.py deleted file mode 100644 index 156520d..0000000 --- a/ckanext/datapackager/plugin/pylons_plugin.py +++ /dev/null @@ -1,27 +0,0 @@ -import ckan.plugins as plugins - -class MixinPlugin(plugins.SingletonPlugin): - plugins.implements(plugins.IRoutes, inherit=True) - - def before_map(self, map_): - map_.connect( - 'import_datapackage', - '/import_datapackage', - controller='ckanext.datapackager.controllers.datapackage:DataPackageController', - action='new', - conditions=dict(method=['GET']), - ) - map_.connect( - 'import_datapackage', - '/import_datapackage', - controller='ckanext.datapackager.controllers.datapackage:DataPackageController', - action='import_datapackage', - conditions=dict(method=['POST']), - ) - map_.connect( - 'export_datapackage', - '/dataset/{package_id}/datapackage.json', - controller='ckanext.datapackager.controllers.datapackage:DataPackageController', - action='export_datapackage' - ) - return map_ From dffe40166e7c347c598238ae5425f416e5a77267 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Fri, 14 Mar 2025 14:13:43 +0800 Subject: [PATCH 02/12] Remove support for old CKAN versions and Python 2 --- .github/workflows/test.yml | 48 ++++++++----------- README.md | 8 +--- .../datapackager/controllers/datapackage.py | 31 +++--------- ckanext/datapackager/logic/action/create.py | 30 ++---------- .../datapackage/import_datapackage.html | 7 +-- .../snippets/import_datapackage_form.html | 2 +- .../templates/organization/read.html | 2 +- .../datapackager/templates/package/read.html | 8 +--- .../templates/package/search.html | 10 +--- .../templates/user/dashboard_datasets.html | 2 +- .../tests/controllers/test_datapackage.py | 46 +++++------------- .../datapackager/tests/lib/test_converter.py | 4 +- .../tests/logic/action/test_create.py | 14 ++---- dev-requirements-py2.txt | 8 ---- dev-requirements.txt | 1 - requirements-py2.txt | 11 ----- requirements.txt | 9 ++-- 17 files changed, 62 insertions(+), 179 deletions(-) delete mode 100644 dev-requirements-py2.txt delete mode 100644 requirements-py2.txt diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fbd5824..b49867d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,10 +4,10 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: - python-version: '3.6' + python-version: '3.9' - name: Install requirements run: pip install flake8 pycodestyle - name: Check syntax @@ -17,16 +17,26 @@ jobs: needs: lint strategy: matrix: - ckan-version: [2.8, 2.9, 2.9-py2] + include: + - ckan-version: "2.11" + ckan-image: "ckan/ckan-dev:2.11-py3.10" + solr-version: "9" + - ckan-version: "2.10" + ckan-image: "ckan/ckan-dev:2.10-py3.10" + solr-version: "9" + - ckan-version: "2.9" + ckan-image: "ckan/ckan-dev:2.9-py3.9" + solr-version: "8" fail-fast: false name: CKAN ${{ matrix.ckan-version }} runs-on: ubuntu-latest container: - image: openknowledge/ckan-dev:${{ matrix.ckan-version }} + image: ${{ matrix.ckan-image }} + options: --user root services: solr: - image: ckan/ckan-solr:${{ matrix.ckan-version }} + image: ckan/ckan-solr:${{ matrix.ckan-version }}-solr${{ matrix.solr-version }} postgres: image: ckan/ckan-postgres-dev:${{ matrix.ckan-version }} env: @@ -44,34 +54,16 @@ jobs: CKAN_REDIS_URL: redis://redis:6379/1 steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v3 - - name: Install requirements (Python 3) - if: ${{ matrix.ckan-version != '2.7' && matrix.ckan-version != '2.8' && matrix.ckan-version != '2.9-py2'}} + - uses: actions/checkout@v4 + - name: Install requirements (common) run: | - pip install cython - pip install cchardet pip install -r requirements.txt pip install -r dev-requirements.txt - - name: Install requirements (Python 2) - if: ${{ matrix.ckan-version == '2.7' || matrix.ckan-version == '2.8' || matrix.ckan-version == '2.9-py2'}} - run: | - pip install cython - pip install cchardet - pip install -r requirements-py2.txt - pip install -r dev-requirements-py2.txt - - name: Install requirements (common) - run: | 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:\/srv\/app\/src\/ckan\/test-core.ini/' test.ini - - name: Setup extension (CKAN >= 2.9) - if: ${{ matrix.ckan-version != '2.7' && matrix.ckan-version != '2.8' }} + - name: Setup extension run: | ckan -c test.ini db init - - name: Setup extension (CKAN < 2.9) - if: ${{ matrix.ckan-version == '2.7' || matrix.ckan-version == '2.8' }} - run: | - paster --plugin=ckan db init -c test.ini - name: Run tests - run: pytest --ckan-ini=test.ini --cov=ckanext.datapackager --cov-report=xml --cov-append --disable-warnings ckanext/datapackager/tests + run: pytest --ckan-ini=test.ini --cov=ckanext.datapackager --cov-report=term-missing --cov-append --disable-warnings ckanext/datapackager/tests diff --git a/README.md b/README.md index 6ace0e5..5d2b9a8 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ This extension adds importing and exporting of [Data Packages][data-packages] to ## Requirements -* CKAN >= 2.8 +* CKAN >= 2.9 ## Installing @@ -128,14 +128,10 @@ For instance You'll need to install the dev requirements to run the tests: -To run the tests on CKAN >= 2.9, do: +To run the tests, do: pytest --ckan-ini=test.ini ckanext/dcat/tests -To run the tests on CKAN <= 2.8, do: - - nosetests --nologcapture --ckan --with-pylons=test-nose.ini ckanext/dcat/tests/nose - Note that ckanext-datapackager's `test.ini` file assumes that the relative path from it to CKAN's `test-core.ini` file is `../ckan/test-core.ini`, i.e. that you have CKAN and ckanext-datapackager installed next to each other in the same directory. This diff --git a/ckanext/datapackager/controllers/datapackage.py b/ckanext/datapackager/controllers/datapackage.py index 7f186fd..2fffc03 100644 --- a/ckanext/datapackager/controllers/datapackage.py +++ b/ckanext/datapackager/controllers/datapackage.py @@ -50,19 +50,15 @@ def import_datapackage(): else: params = toolkit.request.params - if toolkit.check_ckan_version(min_version="2.9"): - if 'upload' in toolkit.request.files: - params['upload'] = toolkit.request.files['upload'] + if 'upload' in toolkit.request.files: + params['upload'] = toolkit.request.files['upload'] dataset = toolkit.get_action('package_create_from_datapackage')( context, params, ) - if toolkit.check_ckan_version(min_version="2.9"): - return toolkit.redirect_to('dataset.read', id=dataset['name']) - else: - return toolkit.redirect_to('dataset_read', id=dataset['name']) + return toolkit.redirect_to('dataset.read', id=dataset['name']) except toolkit.ValidationError as e: errors = e.error_dict @@ -81,7 +77,7 @@ def export_datapackage(package_id): 'user': toolkit.c.user, } - r = make_response() if toolkit.check_ckan_version(min_version="2.9") else toolkit.response + r = make_response() r.content_disposition = 'attachment; filename=datapackage.json'.format( package_id) r.content_type = 'application/json' @@ -94,20 +90,5 @@ def export_datapackage(package_id): except toolkit.ObjectNotFound: return toolkit.abort(404, 'Dataset not found') - if toolkit.check_ckan_version(min_version="2.9"): - r.data = json.dumps(datapackage_dict, indent=2) - return r - else: - return json.dumps(datapackage_dict, indent=2) - - -if not toolkit.check_ckan_version(u'2.9'): - class DataPackageController(toolkit.BaseController): - def new(self, data=None, errors=None, error_summary=None): - return new(data, errors, error_summary) - def import_datapackage(self): - return import_datapackage() - def export_datapackage(self, package_id): - return export_datapackage(package_id) - - + r.data = json.dumps(datapackage_dict, indent=2) + return r diff --git a/ckanext/datapackager/logic/action/create.py b/ckanext/datapackager/logic/action/create.py index b0f057b..9168ee1 100644 --- a/ckanext/datapackager/logic/action/create.py +++ b/ckanext/datapackager/logic/action/create.py @@ -1,10 +1,7 @@ import random -import cgi import json import tempfile -import six - import ckan.plugins.toolkit as toolkit from frictionless_ckan_mapper import frictionless_to_ckan as converter from werkzeug.datastructures import FileStorage @@ -76,7 +73,7 @@ def package_create_from_datapackage(context, data_dict): toolkit.get_action('package_delete')( context, {'id': dataset_id}) except Exception as e2: - six.raise_from(e, e2) + raise e from e2 else: raise e @@ -88,10 +85,7 @@ def _load_and_validate_datapackage(url=None, upload=None): try: if _upload_attribute_is_valid(upload): - if toolkit.check_ckan_version(min_version="2.9"): - dp = datapackage.DataPackage(upload) - else: - dp = datapackage.DataPackage(upload.file) + dp = datapackage.DataPackage(upload) else: dp = datapackage.DataPackage(url) @@ -151,14 +145,11 @@ def _create_and_upload_resource_with_inline_data(context, resource): data = resource['data'] del resource['data'] - if not isinstance(data, six.string_types): + if not isinstance(data, str): data = json.dumps(data, indent=2) with tempfile.NamedTemporaryFile(prefix=prefix) as f: - if six.PY3: - f.write(six.binary_type(data, 'utf-8')) - else: - f.write(six.binary_type(data)) + f.write(bytes(data, 'utf-8')) f.seek(0) _create_and_upload_resource(context, resource, f) @@ -183,21 +174,10 @@ def _create_and_upload_local_resource(context, resource): def _create_and_upload_resource(context, resource, the_file): resource['url'] = 'url' resource['url_type'] = 'upload' - - if toolkit.check_ckan_version(min_version="2.9"): - resource['upload'] = FileStorage(the_file, the_file.name, the_file.name) - else: - resource['upload'] = _UploadLocalFileStorage(the_file) + resource['upload'] = FileStorage(the_file, the_file.name, the_file.name) toolkit.get_action('resource_create')(context, resource) def _upload_attribute_is_valid(upload): return hasattr(upload, 'read') or hasattr(upload, 'file') and hasattr(upload.file, 'read') - -# Used only in CKAN < 2.9 -class _UploadLocalFileStorage(cgi.FieldStorage): - def __init__(self, fp, *args, **kwargs): - self.name = fp.name - self.filename = fp.name - self.file = fp diff --git a/ckanext/datapackager/templates/datapackage/import_datapackage.html b/ckanext/datapackager/templates/datapackage/import_datapackage.html index cc78ad8..a1d8cec 100644 --- a/ckanext/datapackager/templates/datapackage/import_datapackage.html +++ b/ckanext/datapackager/templates/datapackage/import_datapackage.html @@ -3,12 +3,7 @@ {% block subtitle %}{{ _('Import Data Package') }}{% endblock %} {% block breadcrumb_content %} - {% if h.ckan_version().split('.')|map('int')|list >= [2, 9, 0] %} -
  • {% link_for _('Datasets'), named_route='dataset.search' %}
    • - {% else %} -
  • {% link_for _('Datasets'), controller='package', action='search' %}
    • - {% endif %} - +
  • {% link_for _('Datasets'), named_route='dataset.search' %}
  • {{ _('Import Data Package') }}
    • {% endblock %} diff --git a/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html b/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html index 546031c..abc3460 100644 --- a/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html +++ b/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html @@ -1,5 +1,5 @@ {% import 'macros/form.html' as form %} -{% set action = h.url_for('datapackager.import_datapackage') if h.ckan_version().split('.')|map('int')|list >= [2, 9, 0] else h.url_for('import_datapackage') %} +{% set action = h.url_for('datapackager.import_datapackage') %}
    {% block errors %}{{ form.errors(error_summary) }}{% endblock %} diff --git a/ckanext/datapackager/templates/organization/read.html b/ckanext/datapackager/templates/organization/read.html index 2b2b084..9f6ca23 100644 --- a/ckanext/datapackager/templates/organization/read.html +++ b/ckanext/datapackager/templates/organization/read.html @@ -3,7 +3,7 @@ {% block page_primary_action %} {{ super() }} {% if h.check_access('package_create', {'owner_org': c.group_dict.id}) %} - + {{ _('Import Data Package') }} diff --git a/ckanext/datapackager/templates/package/read.html b/ckanext/datapackager/templates/package/read.html index 404c8f1..cbd19f9 100644 --- a/ckanext/datapackager/templates/package/read.html +++ b/ckanext/datapackager/templates/package/read.html @@ -2,13 +2,7 @@ {% block package_resources %}
    - - {% if h.ckan_version().split('.')|map('int')|list >= [2, 9, 0] %} - - {% else %} - - {% endif %} - + Download Data Package
    diff --git a/ckanext/datapackager/templates/package/search.html b/ckanext/datapackager/templates/package/search.html index 52a4882..1f50786 100644 --- a/ckanext/datapackager/templates/package/search.html +++ b/ckanext/datapackager/templates/package/search.html @@ -4,14 +4,8 @@ {% if h.check_access('package_create') %}
    - {% if h.ckan_version().split('.')|map('int')|list >= [2, 9, 0] %} - {% link_for _('Add Dataset'), named_route='dataset.new', class_='btn btn-primary', icon='plus-sign-alt' %} - {% link_for _('Import Data Package'), named_route='datapackager.import_datapackage', class_='btn', icon='plus-sign-alt' %} - {% else %} - {% link_for _('Add Dataset'), controller='package', action='new', class_='btn btn-primary', icon='plus-sign-alt' %} - {% link_for _('Import Data Package'), named_route='import_datapackage', class_='btn', icon='plus-sign-alt' %} - {% endif %} - + {% link_for _('Add Dataset'), named_route='dataset.new', class_='btn btn-primary', icon='plus-sign-alt' %} + {% link_for _('Import Data Package'), named_route='datapackager.import_datapackage', class_='btn', icon='plus-sign-alt' %}
    {% endif %} {% endblock %} diff --git a/ckanext/datapackager/templates/user/dashboard_datasets.html b/ckanext/datapackager/templates/user/dashboard_datasets.html index 7af44c3..d9e9bf1 100644 --- a/ckanext/datapackager/templates/user/dashboard_datasets.html +++ b/ckanext/datapackager/templates/user/dashboard_datasets.html @@ -3,7 +3,7 @@ {% block page_primary_action %} {{ super() }} {% if h.check_access('package_create') %} - + {{ _('Import Data Package') }} diff --git a/ckanext/datapackager/tests/controllers/test_datapackage.py b/ckanext/datapackager/tests/controllers/test_datapackage.py index b5bb3a9..ee1ce40 100644 --- a/ckanext/datapackager/tests/controllers/test_datapackage.py +++ b/ckanext/datapackager/tests/controllers/test_datapackage.py @@ -14,16 +14,6 @@ responses.add_passthru(toolkit.config['solr_url']) -def _url_for(*args, **kwargs): - - if toolkit.check_ckan_version(max_version="2.9"): - if args[0] == "datapackager.export_datapackage": - args = ("export_datapackage", ) - elif args[0] == "datapackager.import_datapackage": - args = ("import_datapackage", ) - - return toolkit.url_for(*args, **kwargs) - @pytest.mark.ckan_config('ckan.plugins', 'datapackager') @pytest.mark.usefixtures('clean_db', 'with_plugins', 'with_request_context') @@ -61,12 +51,13 @@ def test_download_datapackage(self, app): ) # Download the package's JSON file. - url = _url_for('datapackager.export_datapackage', + url = toolkit.url_for('datapackager.export_datapackage', package_id=dataset['name']) response = app.get(url) # Open and validate the response as a JSON. dp = datapackage.DataPackage(json.loads(response.body)) + print(json.loads(response.body)) dp.validate() # Check the contents of the datapackage.json file. @@ -91,24 +82,21 @@ def test_that_download_button_is_on_page(self, app): soup = BeautifulSoup(response.body) download_button = soup.find(id='export_datapackage_button') download_url = download_button['href'] - assert download_url == _url_for('datapackager.export_datapackage', + assert download_url == toolkit.url_for('datapackager.export_datapackage', package_id=dataset['id']) def test_new_renders(self, app): user = factories.User() env = {'REMOTE_USER': user['name'].encode('ascii')} - url = _url_for('datapackager.import_datapackage') + url = toolkit.url_for('datapackager.import_datapackage') response = app.get(url, extra_environ=env) - if toolkit.check_ckan_version(min_version="2.9"): - assert 200 == response.status_code - else: - assert 200 == response.status_int + assert 200 == response.status_code @pytest.mark.ckan_config('ckan.auth.create_unowned_dataset', False) def test_new_requires_user_to_be_able_to_create_packages(self, app): user = factories.User() env = {'REMOTE_USER': user['name'].encode('ascii')} - url = _url_for('datapackager.import_datapackage') + url = toolkit.url_for('datapackager.import_datapackage') response = app.get(url, extra_environ=env, status=401) assert 'Unauthorized to create a dataset' in response.body @@ -128,21 +116,13 @@ def test_import_datapackage(self, app): user = factories.User() env = {'REMOTE_USER': user['name'].encode('ascii')} - url = _url_for('datapackager.import_datapackage', url=datapackage_url) - if toolkit.check_ckan_version(min_version="2.9"): - response = app.post( - url, - extra_environ=env, - follow_redirects=False - ) - - assert response.status_code == 302 - else: - response = app.post( - url, - extra_environ=env, - ) - assert response.status_int == 302 + url = toolkit.url_for('datapackager.import_datapackage', url=datapackage_url) + response = app.post( + url, + extra_environ=env, + follow_redirects=False + ) + assert response.status_code == 302 # Should redirect to dataset's page assert re.match('.*/dataset/foo$', response.headers['Location']) diff --git a/ckanext/datapackager/tests/lib/test_converter.py b/ckanext/datapackager/tests/lib/test_converter.py index 747f3fd..2fe0deb 100644 --- a/ckanext/datapackager/tests/lib/test_converter.py +++ b/ckanext/datapackager/tests/lib/test_converter.py @@ -282,7 +282,7 @@ def test_datapackage_name_title_and_version(self): assert result["name"] == datapackage_dict["name"] assert result["title"] == datapackage_dict["title"] assert result["version"] == datapackage_dict["version"] - + # TODO: check if the name must be lowercased on frictionless-ckan-mapper #def test_name_is_lowercased(self): # self.datapackage.update( @@ -344,7 +344,7 @@ def test_datapackage_author_as_string(self): author = {"name": "John Smith", "email": "jsmith@email.com"} self.datapackage.update({ 'contributors': [{ - "title": author["name"], + "title": author["name"], "email": author["email"], "role": "author" }] diff --git a/ckanext/datapackager/tests/logic/action/test_create.py b/ckanext/datapackager/tests/logic/action/test_create.py index a0bfd39..db15615 100644 --- a/ckanext/datapackager/tests/logic/action/test_create.py +++ b/ckanext/datapackager/tests/logic/action/test_create.py @@ -1,7 +1,6 @@ import json import tempfile -from six import StringIO, BytesIO -import six +from io import StringIO try: from unittest import mock except ImportError: @@ -48,11 +47,7 @@ def test_it_raises_if_datapackage_is_unsafe(self): ] } - if toolkit.check_ckan_version(min_version="2.9"): - upload = StringIO(json.dumps(datapackage)) - else: - upload = mock.MagicMock() - upload.file = StringIO(json.dumps(datapackage)) + upload = StringIO(json.dumps(datapackage)) with pytest.raises(toolkit.ValidationError): helpers.call_action('package_create_from_datapackage', upload=upload) @@ -294,10 +289,7 @@ def test_it_allows_uploading_a_datapackage(self): } with tempfile.NamedTemporaryFile() as tmpfile: - if six.PY3: - tmpfile.write(six.binary_type(json.dumps(datapackage), 'utf-8')) - else: - tmpfile.write(six.binary_type(json.dumps(datapackage))) + tmpfile.write(bytes(json.dumps(datapackage), 'utf-8')) tmpfile.flush() dataset = helpers.call_action('package_create_from_datapackage', diff --git a/dev-requirements-py2.txt b/dev-requirements-py2.txt deleted file mode 100644 index f9a7e74..0000000 --- a/dev-requirements-py2.txt +++ /dev/null @@ -1,8 +0,0 @@ -responses -requests -pytest==4.6.* -pytest-ckan -pytest-cov -pytest-mock -ckanapi -beautifulsoup4 diff --git a/dev-requirements.txt b/dev-requirements.txt index 879a964..b521abe 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,6 +1,5 @@ responses requests -pytest==6.0.0 pytest-ckan pytest-cov pytest-mock diff --git a/requirements-py2.txt b/requirements-py2.txt deleted file mode 100644 index e409825..0000000 --- a/requirements-py2.txt +++ /dev/null @@ -1,11 +0,0 @@ -cython -cchardet -datapackage==1.1.3 -frictionless-ckan-mapper>=1.0.7 -jsonschema==2.6.0 -chardet==4.0.0 -python-slugify==1.2.4 -typing-extensions==3.10.0.2 -rfc3986==1.5.0 -six -ckan-datapackage-tools==0.1.0 diff --git a/requirements.txt b/requirements.txt index e547a3a..140de4a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,6 @@ cython cchardet -jsonschema==2.6.0 -python-slugify==1.2.4 -datapackage==1.1.3 -frictionless-ckan-mapper>=1.0.7 -six +jsonschema==4.23.0 +python-slugify==8.0.4 +datapackage==1.15.4 +frictionless-ckan-mapper==1.0.8 From c429463bcb572d82b41e372ea46530c566368a60 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Tue, 25 Mar 2025 12:04:00 +0800 Subject: [PATCH 03/12] Adapt functional tests to change in auth handling --- .../tests/controllers/test_datapackage.py | 71 +++++++++++++------ 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/ckanext/datapackager/tests/controllers/test_datapackage.py b/ckanext/datapackager/tests/controllers/test_datapackage.py index ee1ce40..7f5f0d2 100644 --- a/ckanext/datapackager/tests/controllers/test_datapackage.py +++ b/ckanext/datapackager/tests/controllers/test_datapackage.py @@ -5,7 +5,6 @@ from bs4 import BeautifulSoup import re -import ckanapi import datapackage import ckan.tests.factories as factories import ckan.tests.helpers as helpers @@ -15,16 +14,41 @@ responses.add_passthru(toolkit.config['solr_url']) +@pytest.fixture +def sysadmin_env(): + try: + from ckan.tests.factories import SysadminWithToken + user = SysadminWithToken() + return {"Authorization": user["token"]} + except ImportError: + # ckan <= 2.9 + from ckan.tests.factories import Sysadmin + user = Sysadmin() + return {"REMOTE_USER": user["name"].encode("ascii")} + + +@pytest.fixture +def user_env(): + try: + from ckan.tests.factories import UserWithToken + user = UserWithToken() + return {"Authorization": user["token"]} + except ImportError: + # ckan <= 2.9 + from ckan.tests.factories import User + user = User() + return {"REMOTE_USER": user["name"].encode("ascii")} + + @pytest.mark.ckan_config('ckan.plugins', 'datapackager') -@pytest.mark.usefixtures('clean_db', 'with_plugins', 'with_request_context') +@pytest.mark.usefixtures('with_plugins', 'clean_db') class TestDataPackageController(): '''Functional tests for the DataPackageController class.''' - def test_download_datapackage(self, app): + def test_download_datapackage(self, app, sysadmin_env): '''Test downloading a DataPackage file of a package. ''' - user = factories.Sysadmin() dataset = factories.Dataset( maintainer = "John Smith", maintainer_email = "jsmith@email.com", @@ -85,23 +109,25 @@ def test_that_download_button_is_on_page(self, app): assert download_url == toolkit.url_for('datapackager.export_datapackage', package_id=dataset['id']) - def test_new_renders(self, app): - user = factories.User() - env = {'REMOTE_USER': user['name'].encode('ascii')} + def test_new_renders(self, app, user_env): url = toolkit.url_for('datapackager.import_datapackage') - response = app.get(url, extra_environ=env) + if toolkit.check_ckan_version(min_version="2.10.0"): + response = app.get(url, headers=user_env) + else: + response = app.get(url, environ_overrides=user_env) assert 200 == response.status_code @pytest.mark.ckan_config('ckan.auth.create_unowned_dataset', False) - def test_new_requires_user_to_be_able_to_create_packages(self, app): - user = factories.User() - env = {'REMOTE_USER': user['name'].encode('ascii')} + def test_new_requires_user_to_be_able_to_create_packages(self, app, user_env): url = toolkit.url_for('datapackager.import_datapackage') - response = app.get(url, extra_environ=env, status=401) + if toolkit.check_ckan_version(min_version="2.10.0"): + response = app.get(url, headers=user_env, status=401) + else: + response = app.get(url, environ_overrides=user_env, status=401) assert 'Unauthorized to create a dataset' in response.body @responses.activate - def test_import_datapackage(self, app): + def test_import_datapackage(self, app, user_env): datapackage_url = 'http://www.foo.com/datapackage.json' datapackage = { 'name': 'foo', @@ -114,14 +140,19 @@ def test_import_datapackage(self, app): } responses.add('GET', datapackage_url, json=datapackage) - user = factories.User() - env = {'REMOTE_USER': user['name'].encode('ascii')} url = toolkit.url_for('datapackager.import_datapackage', url=datapackage_url) - response = app.post( - url, - extra_environ=env, - follow_redirects=False - ) + if toolkit.check_ckan_version(min_version="2.10.0"): + response = app.post( + url, + headers=user_env, + follow_redirects=False + ) + else: + response = app.post( + url, + environ_overrides=user_env, + follow_redirects=False + ) assert response.status_code == 302 # Should redirect to dataset's page From 00543a47befa7235fe9146af54ef61296931f6db Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Tue, 25 Mar 2025 15:23:59 +0800 Subject: [PATCH 04/12] Add csrf_input to the form --- .../templates/datapackage/snippets/import_datapackage_form.html | 1 + 1 file changed, 1 insertion(+) diff --git a/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html b/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html index abc3460..f21633e 100644 --- a/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html +++ b/ckanext/datapackager/templates/datapackage/snippets/import_datapackage_form.html @@ -2,6 +2,7 @@ {% set action = h.url_for('datapackager.import_datapackage') %} + {{ h.csrf_input() if 'csrf_input' in h }} {% block errors %}{{ form.errors(error_summary) }}{% endblock %} {% block basic_fields %} From d0fe6adfce9d25e50091fdd5b0889c6af3396c67 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Tue, 25 Mar 2025 17:29:00 +0800 Subject: [PATCH 05/12] Update CSS classes to support CKAN 2.9 to 2.11 --- .../templates/datapackage/import_datapackage.html | 2 +- ckanext/datapackager/templates/package/read.html | 4 ++-- ckanext/datapackager/templates/package/search.html | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ckanext/datapackager/templates/datapackage/import_datapackage.html b/ckanext/datapackager/templates/datapackage/import_datapackage.html index a1d8cec..2c86b6c 100644 --- a/ckanext/datapackager/templates/datapackage/import_datapackage.html +++ b/ckanext/datapackager/templates/datapackage/import_datapackage.html @@ -9,7 +9,7 @@ {% block info_module %}
    -

    {{ _('What are Data Packages?') }}

    +

    {{ _('What are Data Packages?') }}

    {% trans %} diff --git a/ckanext/datapackager/templates/package/read.html b/ckanext/datapackager/templates/package/read.html index cbd19f9..442bca3 100644 --- a/ckanext/datapackager/templates/package/read.html +++ b/ckanext/datapackager/templates/package/read.html @@ -2,8 +2,8 @@ {% block package_resources %}

    diff --git a/ckanext/datapackager/templates/package/search.html b/ckanext/datapackager/templates/package/search.html index 1f50786..b2d59b7 100644 --- a/ckanext/datapackager/templates/package/search.html +++ b/ckanext/datapackager/templates/package/search.html @@ -4,8 +4,8 @@ {% if h.check_access('package_create') %}
    - {% link_for _('Add Dataset'), named_route='dataset.new', class_='btn btn-primary', icon='plus-sign-alt' %} - {% link_for _('Import Data Package'), named_route='datapackager.import_datapackage', class_='btn', icon='plus-sign-alt' %} + {% snippet 'snippets/add_dataset.html', dataset_type=dataset_type %} + {% link_for _('Import Data Package'), named_route='datapackager.import_datapackage', class_='btn btn-primary', icon='fa fa-upload' %}
    {% endif %} {% endblock %} From 45a2574989192de6c465bd2a693df2b54f869d77 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Wed, 26 Mar 2025 16:24:13 +0800 Subject: [PATCH 06/12] Replace the call to requests.params for requests.args --- ckanext/datapackager/controllers/datapackage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckanext/datapackager/controllers/datapackage.py b/ckanext/datapackager/controllers/datapackage.py index 2fffc03..0e994c0 100644 --- a/ckanext/datapackager/controllers/datapackage.py +++ b/ckanext/datapackager/controllers/datapackage.py @@ -22,7 +22,7 @@ def new(data=None, errors=None, error_summary=None): errors = errors or {} error_summary = error_summary or {} default_data = { - 'owner_org': toolkit.request.params.get('group'), + 'owner_org': toolkit.request.args.get('group'), } data = data or default_data From 4db93e77944d2fd47637dedada6d7b9e88bd1a0f Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Wed, 26 Mar 2025 17:23:32 +0800 Subject: [PATCH 07/12] Use the latest frictionless-ckan-mapper; remove cchardet since datapackage-py >= 1.12 uses chardet by default --- ckanext/datapackager/tests/lib/test_converter.py | 2 ++ requirements.txt | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ckanext/datapackager/tests/lib/test_converter.py b/ckanext/datapackager/tests/lib/test_converter.py index 2fe0deb..7bc9e83 100644 --- a/ckanext/datapackager/tests/lib/test_converter.py +++ b/ckanext/datapackager/tests/lib/test_converter.py @@ -20,6 +20,8 @@ def setUp(self): "title": "Countries GDP", "version": "1.0", "resources": [self.resource_dict], + "license_id": "", + "license_title": "" } def test_basic_dataset_in_setup_is_valid(self): diff --git a/requirements.txt b/requirements.txt index 140de4a..56fa6da 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,4 @@ -cython -cchardet jsonschema==4.23.0 python-slugify==8.0.4 datapackage==1.15.4 -frictionless-ckan-mapper==1.0.8 +frictionless-ckan-mapper>=1.0.7 From 7b011185938b371602a777f8321961541b79ee73 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Thu, 27 Mar 2025 16:00:53 +0800 Subject: [PATCH 08/12] Remove Python 2.7 in the setup.py --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index eae6a94..76b7c13 100644 --- a/setup.py +++ b/setup.py @@ -25,7 +25,6 @@ # Specify the Python versions you support here. In particular, ensure # that you indicate whether you support Python 2, Python 3 or both. - 'Programming Language :: Python :: 2.7', "Programming Language :: Python :: 3", ], keywords='CKAN datapackages FrictionlessData', From e3757dbc2e6883602a29581298c5f494feb4f63b Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Wed, 9 Apr 2025 15:38:18 +0800 Subject: [PATCH 09/12] Remove print --- ckanext/datapackager/tests/controllers/test_datapackage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckanext/datapackager/tests/controllers/test_datapackage.py b/ckanext/datapackager/tests/controllers/test_datapackage.py index 7f5f0d2..a86882d 100644 --- a/ckanext/datapackager/tests/controllers/test_datapackage.py +++ b/ckanext/datapackager/tests/controllers/test_datapackage.py @@ -81,7 +81,6 @@ def test_download_datapackage(self, app, sysadmin_env): # Open and validate the response as a JSON. dp = datapackage.DataPackage(json.loads(response.body)) - print(json.loads(response.body)) dp.validate() # Check the contents of the datapackage.json file. From 5141684cd3899d79fac7cdac6136bbdd1a50881f Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Wed, 16 Apr 2025 17:21:31 +0800 Subject: [PATCH 10/12] Fix uploading via link --- ckanext/datapackager/controllers/datapackage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ckanext/datapackager/controllers/datapackage.py b/ckanext/datapackager/controllers/datapackage.py index 0e994c0..465034f 100644 --- a/ckanext/datapackager/controllers/datapackage.py +++ b/ckanext/datapackager/controllers/datapackage.py @@ -51,7 +51,8 @@ def import_datapackage(): params = toolkit.request.params if 'upload' in toolkit.request.files: - params['upload'] = toolkit.request.files['upload'] + if toolkit.request.files['upload'].filename: + params['upload'] = toolkit.request.files['upload'] dataset = toolkit.get_action('package_create_from_datapackage')( context, From 71fb1dd5faedeabfd07baf5695e16f467c4e907a Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Fri, 18 Apr 2025 11:56:17 +0800 Subject: [PATCH 11/12] Remove unused imports --- ckanext/datapackager/tests/logic/action/test_create.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ckanext/datapackager/tests/logic/action/test_create.py b/ckanext/datapackager/tests/logic/action/test_create.py index db15615..820d25a 100644 --- a/ckanext/datapackager/tests/logic/action/test_create.py +++ b/ckanext/datapackager/tests/logic/action/test_create.py @@ -1,10 +1,6 @@ import json import tempfile from io import StringIO -try: - from unittest import mock -except ImportError: - import mock import pytest import responses From c2e26ba000c4d3e791842a74e5edbdcd9882e161 Mon Sep 17 00:00:00 2001 From: Sol Lee Date: Wed, 18 Mar 2026 16:15:41 +0800 Subject: [PATCH 12/12] Fix uploading via file --- ckanext/datapackager/logic/action/create.py | 22 ++++++++++--------- .../tests/controllers/test_datapackage.py | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ckanext/datapackager/logic/action/create.py b/ckanext/datapackager/logic/action/create.py index 9168ee1..c4a5dca 100644 --- a/ckanext/datapackager/logic/action/create.py +++ b/ckanext/datapackager/logic/action/create.py @@ -65,7 +65,7 @@ def package_create_from_datapackage(context, data_dict): if resources: try: - _create_resources(dataset_id, context, resources) + _create_resources(dataset_id, context, resources, dp.resources) res = toolkit.get_action('package_show')( context, {'id': dataset_id}) except Exception as e: @@ -126,18 +126,20 @@ def _package_create_with_unique_name(context, dataset_dict, name=None): return res -def _create_resources(dataset_id, context, resources): +def _create_resources(dataset_id, context, resources, dp_resources): for resource in resources: resource['package_id'] = dataset_id if resource.get('data'): _create_and_upload_resource_with_inline_data(context, resource) - elif resource.get('path'): - _create_and_upload_local_resource(context, resource) - else: - # TODO: Investigate why in test_controller the resource['url'] is a list - if type(resource['url']) is list: - resource['url'] = resource['url'][0] - toolkit.get_action('resource_create')(context, resource) + continue + for dp_res in dp_resources: + if dp_res.descriptor['path'] != resource['url']: + continue + elif dp_res.local: + resource['path'] = dp_res.source + _create_and_upload_local_resource(context, resource) + else: + toolkit.get_action('resource_create')(context, resource) def _create_and_upload_resource_with_inline_data(context, resource): @@ -161,7 +163,7 @@ def _create_and_upload_local_resource(context, resource): if isinstance(path, list): path = path[0] try: - with open(path, 'r') as f: + with open(path, 'rb') as f: _create_and_upload_resource(context, resource, f) except IOError: msg = {'datapackage': [( diff --git a/ckanext/datapackager/tests/controllers/test_datapackage.py b/ckanext/datapackager/tests/controllers/test_datapackage.py index a86882d..b7f2f05 100644 --- a/ckanext/datapackager/tests/controllers/test_datapackage.py +++ b/ckanext/datapackager/tests/controllers/test_datapackage.py @@ -133,7 +133,7 @@ def test_import_datapackage(self, app, user_env): 'resources': [ { 'name': 'the-resource', - 'url': 'http://www.somewhere.com/data.csv', + 'path': 'http://www.somewhere.com/data.csv', } ] } @@ -162,4 +162,4 @@ def test_import_datapackage(self, app, user_env): assert dataset['name'] == 'foo' assert len(dataset.get('resources', [])) == 1 assert dataset['resources'][0].get('name') == 'the-resource' - assert (dataset['resources'][0].get('url') == datapackage['resources'][0]['url']) + assert (dataset['resources'][0].get('url') == datapackage['resources'][0]['path'])