diff --git a/.github/actions/gen-report/action.yml b/.github/actions/gen-report/action.yml new file mode 100644 index 00000000000..75220e8032a --- /dev/null +++ b/.github/actions/gen-report/action.yml @@ -0,0 +1,15 @@ +name: 'Generate Report' +description: 'Generate test report from junit xml file' +runs: + using: "composite" + steps: + - name: Generate Report + uses: dorny/test-reporter@v1 + if: success() || failure() # run this step even if previous step failed + with: + name: ${{github.job}} REPORT # Name of the check run which will be created + path: report.xml # Path to test results + reporter: java-junit # Format of test results + max-annotations: 49 + list-tests: failed + list-suites: failed diff --git a/.github/actions/start-build/action.yml b/.github/actions/start-build/action.yml index 46dffc474c9..ea4fa0ff66a 100644 --- a/.github/actions/start-build/action.yml +++ b/.github/actions/start-build/action.yml @@ -3,6 +3,27 @@ description: 'Last composite action before tests are run' runs: using: "composite" steps: + - id: cache-objects + uses: actions/cache@v2 + with: + path: ~/.cache + key: reqs_${{ hashFiles('poetry.lock') }} + restore-keys: reqs_ + - uses: ./.github/actions/build-es + with: + ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} + - uses: ./.github/actions/build-es6 + with: + ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} + - name: Set up Python 3.12 + uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Install lxml + shell: bash + run: | + sudo apt update + sudo apt-get install libxml2-dev libxslt-dev python-dev - name: Copy Settings shell: bash run: | @@ -13,19 +34,15 @@ runs: - name: PIP install shell: bash run: | - pip install --upgrade pip - pip install invoke==0.13.0 - pip install flake8==2.4.0 --force-reinstall --upgrade - pip install wheel - invoke wheelhouse --dev --addons + # pinned pip to 21.0a and setuptools<58.0.0 as current dependencies do not build on newer versions of pip and setuptools + pip install poetry==1.8.0 + poetry install --no-root --without release - name: Other installs shell: bash run: | - pip install psycopg2==2.7.3 --no-binary psycopg2 - invoke travis_addon_settings - invoke requirements --dev --addons + # bumped psycopg to match requirements.txt, as otherwise build would fail + poetry run python3 -m invoke travis-addon-settings pip uninstall uritemplate.py --yes - pip install uritemplate.py==0.3.0 # use yarn add --exact to match versions in yarn.lock w/o installing all deps yarn add --exact bower@^1.8.8 yarn add --exact @centerforopenscience/list-of-licenses@^1.1.0 diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index bee350262b7..7fe875888d9 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -21,7 +21,7 @@ jobs: uses: actions/cache@v2 with: path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} + key: reqs_${{ hashFiles('**/pyproject.toml') }} restore-keys: reqs - run: | mkdir -p ~/.cache/downloads @@ -33,10 +33,11 @@ jobs: addons: runs-on: ubuntu-20.04 needs: build-cache + permissions: + checks: write services: postgres: image: postgres - env: POSTGRES_PASSWORD: postgres options: >- @@ -49,37 +50,21 @@ jobs: - 5432:5432 steps: - uses: actions/checkout@v2 - - id: cache-objects - uses: actions/cache@v2 - with: - path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} - restore-keys: reqs_ - - uses: ./.github/actions/build-es - with: - ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} - - uses: ./.github/actions/build-es6 - with: - ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} - - name: Set up Python 3.6 - uses: actions/setup-python@v2 - with: - python-version: 3.6 - - name: Install lxml - run: | - sudo apt update - sudo apt-get install libxml2-dev libxslt-dev python-dev - uses: ./.github/actions/start-build - name: Run tests - run: invoke test_travis_addons -n 1 + run: poetry run python3 -m invoke test-travis-addons -n 1 --junit + - name: Upload report + if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed + uses: ./.github/actions/gen-report website: runs-on: ubuntu-20.04 needs: build-cache + permissions: + checks: write services: postgres: image: postgres - env: POSTGRES_PASSWORD: postgres options: >- @@ -92,37 +77,21 @@ jobs: - 5432:5432 steps: - uses: actions/checkout@v2 - - id: cache-objects - uses: actions/cache@v2 - with: - path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} - restore-keys: reqs_ - - uses: ./.github/actions/build-es - with: - ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} - - uses: ./.github/actions/build-es6 - with: - ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} - - name: Set up Python 3.6 - uses: actions/setup-python@v2 - with: - python-version: 3.6 - - name: Install lxml - run: | - sudo apt update - sudo apt-get install libxml2-dev libxslt-dev python-dev - uses: ./.github/actions/start-build - name: Run tests - run: invoke test_travis_website -n 1 + run: poetry run python3 -m invoke test-travis-website -n 1 --junit + - name: Upload report + if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed + uses: ./.github/actions/gen-report api1_and_js: runs-on: ubuntu-20.04 needs: build-cache + permissions: + checks: write services: postgres: image: postgres - env: POSTGRES_PASSWORD: postgres options: >- @@ -135,40 +104,23 @@ jobs: - 5432:5432 steps: - uses: actions/checkout@v2 - - id: cache-objects - uses: actions/cache@v2 - with: - path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} - restore-keys: reqs_ - - uses: ./.github/actions/build-es - with: - ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} - - uses: ./.github/actions/build-es6 - with: - ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} - - name: Set up Python 3.6 - uses: actions/setup-python@v2 - with: - python-version: 3.6 - - name: Install lxml - run: | - sudo apt update - sudo apt-get install libxml2-dev libxslt-dev python-dev - uses: ./.github/actions/start-build - name: NVM & yarn install - run: | - invoke assets --dev + run: poetry run python3 -m invoke assets --dev - name: Run test - run: invoke test_travis_api1_and_js -n 1 + run: poetry run python3 -m invoke test-travis-api1-and-js -n 1 --junit + - name: Upload report + if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed + uses: ./.github/actions/gen-report api2: runs-on: ubuntu-20.04 needs: build-cache + permissions: + checks: write services: postgres: image: postgres - env: POSTGRES_PASSWORD: postgres options: >- @@ -181,32 +133,17 @@ jobs: - 5432:5432 steps: - uses: actions/checkout@v2 - - id: cache-objects - uses: actions/cache@v2 - with: - path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} - restore-keys: reqs_ - - uses: ./.github/actions/build-es - with: - ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} - - uses: ./.github/actions/build-es6 - with: - ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} - - name: Set up Python 3.6 - uses: actions/setup-python@v2 - with: - python-version: 3.6 - - name: Install lxml - run: | - sudo apt update - sudo apt-get install libxml2-dev libxslt-dev python-dev - uses: ./.github/actions/start-build - name: Run tests - run: invoke test_travis_api2 -n 1 + run: poetry run python3 -m invoke test-travis-api2 -n 1 --junit + - name: Upload report + if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed + uses: ./.github/actions/gen-report api3_and_osf: runs-on: ubuntu-20.04 + permissions: + checks: write needs: build-cache services: postgres: @@ -224,27 +161,9 @@ jobs: - 5432:5432 steps: - uses: actions/checkout@v2 - - id: cache-objects - uses: actions/cache@v2 - with: - path: ~/.cache - key: reqs_${{ hashFiles('**/requirements.txt') }} - restore-keys: reqs_ - - uses: ./.github/actions/build-es - with: - ELASTICSEARCH_ARCHIVE: ${{ env.ELASTICSEARCH_ARCHIVE }} - - uses: ./.github/actions/build-es6 - with: - ELASTICSEARCH6_ARCHIVE: ${{ env.ELASTICSEARCH6_ARCHIVE }} - - name: Set up Python 3.6 - uses: actions/setup-python@v2 - with: - python-version: 3.6 - - name: Install lxml - run: | - sudo apt update - sudo apt-get install libxml2-dev libxslt-dev python-dev - uses: ./.github/actions/start-build - name: Run tests - run: invoke test_travis_api3_and_osf -n 1 - + run: poetry run python3 -m invoke test-travis-api3-and-osf -n 1 --junit + - name: Upload report + if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed + uses: ./.github/actions/gen-report diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 718049378df..710f0b86983 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,21 +1,24 @@ default_language_version: - python: python3.6 + python: python3.12 repos: - repo: https://github.com/asottile/add-trailing-comma - rev: v0.7.0 + rev: v3.1.0 hooks: - id: add-trailing-comma # TODO: Remove this line. For now, we only format the api/ directory files: ^api/ - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v1.4.0 + rev: v4.6.0 hooks: - id: double-quote-string-fixer - id: trailing-whitespace exclude: "^(website/static/vendor/|osf/metadata/definitions/.*\\.xsd)" - - id: flake8 - additional_dependencies: ["flake8==3.6.0", "flake8-mutable==1.2.0"] +- repo: https://github.com/pycqa/flake8 + rev: '7.0.0' # pick a git hash / tag to point to + hooks: + - id: flake8 + additional_dependencies: ["flake8-mutable==1.2.0"] - repo: https://github.com/pre-commit/mirrors-jshint - rev: v2.9.6 + rev: v2.13.6 hooks: - id: jshint diff --git a/.travis.yml b/.travis.yml index c769b34141f..0d5c765066d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: python python: - - "3.6" + - "3.10" dist: trusty @@ -132,7 +132,7 @@ install: - travis_retry pip install --upgrade pip - travis_retry pip install invoke==0.13.0 - - travis_retry pip install flake8==2.4.0 --force-reinstall --upgrade + - travis_retry pip install flake8==7.0.0 --force-reinstall --upgrade - travis_retry invoke wheelhouse --dev --addons - | @@ -145,7 +145,8 @@ install: fi - travis_retry invoke travis_addon_settings - - travis_retry pip install psycopg2==2.7.3 --no-binary psycopg2 + # bumped psycopg to match requirements.txt, as otherwise build would fail + - travis_retry pip install psycopg2==2.9.9 --no-binary psycopg2 - travis_retry invoke requirements --dev --addons # Hack to fix package conflict between uritemplate and uritemplate.py (dependency of github3.py) - pip uninstall uritemplate.py --yes diff --git a/CHANGELOG b/CHANGELOG index 1db53b2d592..ea0a600ebd7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,29 @@ We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO. -24.01.0 (2024-04-30) +24.05.0 (2024-07-22) +==================== +- Bump base python version from py3.6 to py3.12. +- Switch to using poetry for dependency management +- Bump most (not all) dependencies to their maximum version as of mid-March. +- Significantly update Dockerfile +- Upgrade Django to v4.2 +- Generate test summary reports in CI + +24.04.0 (2024-07-08) +==================== +- Preprints into Ember OSF Web, Phase 2 Backend Release + +24.03.0 (2024-07-08) +==================== +- Allow AbstractProviders to specify Discover page presence +- Update get_auth +- Expand Addons Service support functionality, waffled +- Set default Resource Type General for Registrations +- Bug fix: Archiver Unicode edge cases +- Bug fix: RelationshipField during project creation + +24.02.0 (2024-04-30) ==================== - Initial Addons Service work, Waffled - Improve default `description` for Google Dataset Discovery diff --git a/Dockerfile b/Dockerfile index ace0492965a..189b0e998b9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,85 +1,61 @@ -FROM node:8-alpine3.9 +FROM python:3.12-alpine3.17 AS base -# Source: https://github.com/docker-library/httpd/blob/7976cabe162268bd5ad2d233d61e340447bfc371/2.4/alpine/Dockerfile#L3 +# Creation of www-data group was removed as it is created by default in alpine 3.14 and higher +# Alpine does not create a www-data user, so we still need to create that. 82 is the standard +# uid/guid for www-data in Alpine. RUN set -x \ - && addgroup -g 82 -S www-data \ && adduser -h /var/www -u 82 -D -S -G www-data www-data RUN apk add --no-cache --virtual .run-deps \ gcc \ g++ \ - python3-dev \ + nodejs \ + npm \ + yarn \ libxslt-dev \ su-exec \ bash \ - python3 \ git \ - # lxml2 libxml2 \ libxslt \ - # psycopg2 - postgresql-libs \ - # cryptography + libpq-dev \ libffi \ - # gevent libev \ libevent \ - && yarn global add bower + && yarn global add bower \ + && mkdir -p /var/www \ + && chown www-data:www-data /var/www -RUN python3 -m ensurepip && \ - pip3 install --upgrade pip==21.0 +ENV POETRY_NO_INTERACTION=1 \ + POETRY_VIRTUALENVS_OPTIONS_ALWAYS_COPY=1 \ + POETRY_VIRTUALENVS_CREATE=0 -WORKDIR /code +FROM base AS build + +ENV POETRY_VIRTUALENVS_IN_PROJECT=1 \ + YARN_CACHE_FOLDER=/tmp/yarn-cache \ + POETRY_CACHE_DIR=/tmp/poetry-cache \ + POETRY_HOME=/tmp/poetry + +RUN python3 -m venv $POETRY_HOME +RUN $POETRY_HOME/bin/pip install poetry==1.8.3 -COPY ./requirements.txt ./ -COPY ./requirements/ ./requirements/ -COPY ./addons/bitbucket/requirements.txt ./addons/bitbucket/ -COPY ./addons/boa/requirements.txt ./addons/boa/ -COPY ./addons/box/requirements.txt ./addons/box/ -#COPY ./addons/citations/requirements.txt ./addons/citations/ -COPY ./addons/dataverse/requirements.txt ./addons/dataverse/ -COPY ./addons/dropbox/requirements.txt ./addons/dropbox/ -#COPY ./addons/figshare/requirements.txt ./addons/figshare/ -#COPY ./addons/forward/requirements.txt ./addons/forward/ -COPY ./addons/github/requirements.txt ./addons/github/ -COPY ./addons/gitlab/requirements.txt ./addons/gitlab/ -#COPY ./addons/googledrive/requirements.txt ./addons/googledrive/ -COPY ./addons/mendeley/requirements.txt ./addons/mendeley/ -COPY ./addons/onedrive/requirements.txt /code/addons/onedrive/ -#COPY ./addons/osfstorage/requirements.txt ./addons/osfstorage/ -COPY ./addons/owncloud/requirements.txt ./addons/owncloud/ -COPY ./addons/s3/requirements.txt ./addons/s3/ -COPY ./addons/twofactor/requirements.txt ./addons/twofactor/ -#COPY ./addons/wiki/requirements.txt ./addons/wiki/ -COPY ./addons/zotero/requirements.txt ./addons/zotero/ RUN set -ex \ - && mkdir -p /var/www \ - && chown www-data:www-data /var/www \ && apk add --no-cache --virtual .build-deps \ build-base \ linux-headers \ - python3-dev \ - # lxml2 musl-dev \ libxml2-dev \ libxslt-dev \ - # psycopg2 - postgresql-dev \ # cryptography - libffi-dev \ - && for reqs_file in \ - /code/requirements.txt \ - /code/requirements/release.txt \ - /code/addons/*/requirements.txt \ - ; do \ - pip3 install --no-cache-dir -r "$reqs_file" \ - ; done \ - && (pip3 uninstall uritemplate.py --yes || true) \ - && pip3 install --no-cache-dir uritemplate.py==0.3.0 \ - # Fix: https://github.com/CenterForOpenScience/osf.io/pull/6783 - && python3 -m compileall /usr/lib/python3.6 || true \ - && apk del .build-deps + libffi-dev + +WORKDIR /code +COPY pyproject.toml . +COPY poetry.lock . +# Fix: https://github.com/CenterForOpenScience/osf.io/pull/6783 +RUN $POETRY_HOME/bin/poetry install --without=dev --no-root --compile # Settings COPY ./tasks/ ./tasks/ @@ -137,7 +113,7 @@ RUN \ # OSF yarn install --frozen-lockfile \ && mkdir -p ./website/static/built/ \ - && invoke build_js_config_files \ + && python3 -m invoke build-js-config-files \ && yarn run webpack-prod \ # Admin && cd ./admin \ @@ -152,23 +128,32 @@ RUN \ COPY ./ ./ ARG GIT_COMMIT= -ENV GIT_COMMIT ${GIT_COMMIT} +ENV GIT_COMMIT=${GIT_COMMIT} -# TODO: Admin/API should fully specify their bower static deps, and not include ./website/static in their defaults.py. +# TODO: Admin/API should fully specify their bower static deps, and not +# include ./website/static in their defaults.py. # (this adds an additional 300+mb to the build image) + RUN for module in \ - api.base.settings \ - admin.base.settings \ - ; do \ - export DJANGO_SETTINGS_MODULE=$module \ - && python3 manage.py collectstatic --noinput --no-init-app \ - ; done \ - && for file in \ - ./website/templates/_log_templates.mako \ - ./website/static/built/nodeCategories.json \ - ; do \ - touch $file && chmod o+w $file \ - ; done \ - && rm ./website/settings/local.py ./api/base/settings/local.py - -CMD ["su-exec", "nobody", "invoke", "--list"] + api.base.settings \ + admin.base.settings \ + ; do \ + export DJANGO_SETTINGS_MODULE=$module \ + && python3 manage.py collectstatic --noinput --no-init-app \ + ; done \ + && for file in \ + ./website/templates/_log_templates.mako \ + ./website/static/built/nodeCategories.json \ + ; do \ + touch $file && chmod o+w $file \ + ; done \ + && rm ./website/settings/local.py ./api/base/settings/local.py + +FROM base AS runtime + +WORKDIR /code +COPY --from=build /usr/local/lib/python3.12 /usr/local/lib/python3.12 +COPY --from=build /usr/local/bin /usr/local/bin +COPY --from=build /code /code + +CMD ["su-exec", "nobody", "python", "-m", "invoke", "--list"] diff --git a/README-docker-compose.md b/README-docker-compose.md index b4824d7768b..9d8d6a2ba46 100644 --- a/README-docker-compose.md +++ b/README-docker-compose.md @@ -339,7 +339,7 @@ resetting docker. To back up your database, follow the following sequence of com 1. Install Postgres on your local machine, outside of docker. (eg `brew install postgres`) To avoid migrations, the version you install must match the one used by the docker container. - ([as of this writing](https://github.com/CenterForOpenScience/osf.io/blob/ce1702cbc95eb7777e5aaf650658a9966f0e6b0c/docker-compose.yml#L53), Postgres 9.6) + ([as of this writing](https://github.com/CenterForOpenScience/osf.io/blob/ce1702cbc95eb7777e5aaf650658a9966f0e6b0c/docker-compose.yml#L53), Postgres 15) 2. Start postgres locally. This must be on a different port than the one used by [docker postgres](https://github.com/CenterForOpenScience/osf.io/blob/ce1702cbc95eb7777e5aaf650658a9966f0e6b0c/docker-compose.yml#L61). Eg, `pg_ctl -D /usr/local/var/postgres start -o "-p 5433"` 3. Verify that the postgres docker container is running (`docker-compose up -d postgres`) @@ -350,8 +350,26 @@ resetting docker. To back up your database, follow the following sequence of com (shorthand: `pg_dump -U postgres -Z 9 -C --c -Fd --j 4 -h localhost --f ~/Desktop/osf_backup osf`) +### Migration of Postgres from 9.6 to 15 version +1. Dumping the database from the existing postgres 9.6 container. +```bash + docker exec -i osfio-postgres-1 /bin/bash -c "pg_dump --username postgres osf" > ./dump.sql +``` +2. Delete a persistent storage volume: + **WARNING: All postgres data will be destroyed.** + - `$ docker-compose stop -t 0 postgres` + - `$ docker-compose rm postgres` + - `$ docker volume rm osfio_postgres_data_vol` +3. Starting a new postgres container. +```bash +docker-compose up -d postgres +``` +4. Restoring the database from the dump file into the new postgres container. +```bash +docker exec -i osfio-postgres-1 /bin/bash -c "psql --username postgres osf" < ./dump.sql +``` -#### Restoring your database +### Restoring your database To restore a local copy of your database for use inside docker, make sure to start both local and dockerized postgres (as shown above). For best results, start from a clean postgres container with no other data. (see below for instructions on dropping postgres data volumes) diff --git a/addons/base/apps.py b/addons/base/apps.py index b9e5a513b52..66038377f9a 100644 --- a/addons/base/apps.py +++ b/addons/base/apps.py @@ -45,7 +45,7 @@ def _root_folder(node_settings, auth, **kwargs): private_key=kwargs.get('view_only', None), ) return [root] - _root_folder.__name__ = '{0}_root_folder'.format(addon_short_name) + _root_folder.__name__ = f'{addon_short_name}_root_folder' return _root_folder @@ -74,7 +74,7 @@ class BaseAddonAppConfig(AppConfig): categories = [] def __init__(self, *args, **kwargs): - ret = super(BaseAddonAppConfig, self).__init__(*args, **kwargs).__init__() + ret = super().__init__(*args, **kwargs).__init__() # Build template lookup paths = [settings.TEMPLATES_PATH] if self.user_settings_template: @@ -82,13 +82,11 @@ def __init__(self, *args, **kwargs): if self.node_settings_template: paths.append(os.path.dirname(self.node_settings_template)) template_dirs = list( - set( - [ - path - for path in paths - if os.path.exists(path) - ] - ) + { + path + for path in paths + if os.path.exists(path) + } ) if template_dirs: self.template_lookup = TemplateLookup( diff --git a/addons/base/generic_views.py b/addons/base/generic_views.py index 18ef2590b28..d7eb4da6ced 100644 --- a/addons/base/generic_views.py +++ b/addons/base/generic_views.py @@ -39,7 +39,7 @@ def _import_auth(auth, node_addon, user_addon, **kwargs): 'result': Serializer().serialize_settings(node_addon, auth.user), 'message': 'Successfully imported credentials from profile.', } - _import_auth.__name__ = '{0}_import_auth'.format(addon_short_name) + _import_auth.__name__ = f'{addon_short_name}_import_auth' return _import_auth @@ -49,7 +49,7 @@ def _account_list(auth): user_settings = auth.user.get_addon(addon_short_name) serializer = Serializer(user_settings=user_settings) return serializer.serialized_user_settings - _account_list.__name__ = '{0}_account_list'.format(addon_short_name) + _account_list.__name__ = f'{addon_short_name}_account_list' return _account_list @@ -64,7 +64,7 @@ def _folder_list(node_addon, **kwargs): folder_id = request.args.get('folderId') return get_folders(node_addon, folder_id) - _folder_list.__name__ = '{0}_folder_list'.format(addon_short_name) + _folder_list.__name__ = f'{addon_short_name}_folder_list' return _folder_list @@ -81,7 +81,7 @@ def _get_config(node_addon, auth, **kwargs): auth.user ) } - _get_config.__name__ = '{0}_get_config'.format(addon_short_name) + _get_config.__name__ = f'{addon_short_name}_get_config' return _get_config @@ -100,16 +100,14 @@ def _set_config(node_addon, user_addon, auth, **kwargs): return { 'result': { 'folder': { - 'name': path.replace('All Files', '') if path != '/' else '/ (Full {0})'.format( - addon_full_name - ), + 'name': path.replace('All Files', '') if path != '/' else f'/ (Full {addon_full_name})', 'path': path, }, 'urls': Serializer(node_settings=node_addon).addon_serialized_urls, }, 'message': 'Successfully updated settings.', } - _set_config.__name__ = '{0}_set_config'.format(addon_short_name) + _set_config.__name__ = f'{addon_short_name}_set_config' return _set_config @@ -120,5 +118,5 @@ def deauthorize_node(addon_short_name): def _deauthorize_node(auth, node_addon, **kwargs): node_addon.deauthorize(auth=auth) node_addon.save() - _deauthorize_node.__name__ = '{0}_deauthorize_node'.format(addon_short_name) + _deauthorize_node.__name__ = f'{addon_short_name}_deauthorize_node' return _deauthorize_node diff --git a/addons/base/logger.py b/addons/base/logger.py index a09abc15df0..edb80e25098 100644 --- a/addons/base/logger.py +++ b/addons/base/logger.py @@ -1,6 +1,6 @@ import abc -class AddonNodeLogger(object): +class AddonNodeLogger: """Helper class for adding correctly-formatted addon logs to nodes. :param Node node: The node to add logs to @@ -8,7 +8,8 @@ class AddonNodeLogger(object): """ __metaclass__ = abc.ABCMeta - @abc.abstractproperty + @property + @abc.abstractmethod def addon_short_name(self): pass @@ -53,7 +54,7 @@ def log(self, action, extra=None, save=False): params.update(extra) self.node.add_log( - action='{0}_{1}'.format(self.addon_short_name, action), + action=f'{self.addon_short_name}_{action}', params=params, auth=self.auth ) diff --git a/addons/base/models.py b/addons/base/models.py index 60d26d9b314..506b7709357 100644 --- a/addons/base/models.py +++ b/addons/base/models.py @@ -119,7 +119,7 @@ def can_be_merged(self): return hasattr(self, 'merge') def to_json(self, user): - ret = super(BaseUserSettings, self).to_json(user) + ret = super().to_json(user) ret['has_auth'] = self.has_auth ret.update({ 'nodes': [ @@ -137,8 +137,8 @@ def to_json(self, user): def __repr__(self): if self.owner: - return '<{cls} owned by user {uid}>'.format(cls=self.__class__.__name__, uid=self.owner._id) - return '<{cls} with no owner>'.format(cls=self.__class__.__name__) + return f'<{self.__class__.__name__} owned by user {self.owner._id}>' + return f'<{self.__class__.__name__} with no owner>' @oauth_complete.connect @@ -186,7 +186,7 @@ def external_accounts(self): def delete(self, save=True): for account in self.external_accounts.filter(provider=self.config.short_name): self.revoke_oauth_access(account, save=False) - super(BaseOAuthUserSettings, self).delete(save=save) + super().delete(save=save) def grant_oauth_access(self, node, external_account, metadata=None): """Give a node permission to use an ``ExternalAccount`` instance.""" @@ -328,7 +328,7 @@ def merge(self, user_settings): self.save() def to_json(self, user): - ret = super(BaseOAuthUserSettings, self).to_json(user) + ret = super().to_json(user) ret['accounts'] = self.serializer( user_settings=self @@ -343,7 +343,7 @@ def to_json(self, user): def on_delete(self): """When the user deactivates the addon, clear auth for connected nodes. """ - super(BaseOAuthUserSettings, self).on_delete() + super().on_delete() nodes = [AbstractNode.load(node_id) for node_id in self.oauth_grants.keys()] for node in nodes: node_addon = node.get_addon(self.oauth_provider.short_name) @@ -382,7 +382,7 @@ def display_name(self): return self.short_name def to_json(self, user): - ret = super(BaseNodeSettings, self).to_json(user) + ret = super().to_json(user) ret.update({ 'user': { 'permissions': self.owner.get_permissions(user) @@ -462,32 +462,27 @@ def before_fork(self, node, user): if hasattr(self, 'user_settings'): if self.user_settings is None: return ( - u'Because you have not configured the {addon} add-on, your authentication will not be ' - u'transferred to the forked {category}. You may authorize and configure the {addon} add-on ' - u'in the new fork on the settings page.' - ).format( - addon=self.config.full_name, - category=node.project_or_component, + f'Because you have not configured the {self.config.full_name} ' + 'add-on, your authentication will not be transferred to the forked ' + f'{node.project_or_component}. You may authorize and configure the ' + f'{self.config.full_name} add-on in the new fork on the settings ' + 'page.' ) elif self.user_settings and self.user_settings.owner == user: return ( - u'Because you have authorized the {addon} add-on for this ' - u'{category}, forking it will also transfer your authentication to ' - u'the forked {category}.' - ).format( - addon=self.config.full_name, - category=node.project_or_component, + f'Because you have authorized the {self.config.full_name} add-on ' + f'for this {node.project_or_component}, forking it will also ' + 'transfer your authentication to the forked ' + f'{node.project_or_component}.' ) else: return ( - u'Because the {addon} add-on has been authorized by a different ' - u'user, forking it will not transfer authentication to the forked ' - u'{category}. You may authorize and configure the {addon} add-on ' - u'in the new fork on the settings page.' - ).format( - addon=self.config.full_name, - category=node.project_or_component, + f'Because the {self.config.full_name} add-on has been authorized ' + 'by a different user, forking it will not transfer authentication ' + f'to the forked {node.project_or_component}. You may authorize and ' + f'configure the {self.config.full_name} add-on in the new fork on ' + 'the settings page.' ) def after_fork(self, node, fork, user, save=True): @@ -545,12 +540,12 @@ def after_delete(self, user): # Archiver # ############ -class GenericRootNode(object): +class GenericRootNode: path = '/' name = '' -class BaseStorageAddon(object): +class BaseStorageAddon: """ Mixin class for traversing file trees of addons with files """ @@ -562,10 +557,10 @@ class Meta: @property def archive_folder_name(self): - name = 'Archive of {addon}'.format(addon=self.config.full_name) + name = f'Archive of {self.config.full_name}' folder_name = getattr(self, 'folder_name', '').lstrip('/').strip() if folder_name: - name = name + ': {folder}'.format(folder=folder_name) + name = name + f': {folder_name}' return name def _get_fileobj_child_metadata(self, filenode, user, cookie=None, version=None): @@ -644,19 +639,22 @@ class BaseOAuthNodeSettings(BaseNodeSettings): class Meta: abstract = True - @abc.abstractproperty + @property + @abc.abstractmethod def folder_id(self): raise NotImplementedError( "BaseOAuthNodeSettings subclasses must expose a 'folder_id' property." ) - @abc.abstractproperty + @property + @abc.abstractmethod def folder_name(self): raise NotImplementedError( "BaseOAuthNodeSettings subclasses must expose a 'folder_name' property." ) - @abc.abstractproperty + @property + @abc.abstractmethod def folder_path(self): raise NotImplementedError( "BaseOAuthNodeSettings subclasses must expose a 'folder_path' property." @@ -674,7 +672,7 @@ def nodelogger(self): self, '_logger_class', type( - '{0}NodeLogger'.format(self.config.short_name.capitalize()), + f'{self.config.short_name.capitalize()}NodeLogger', (logger.AddonNodeLogger,), {'addon_short_name': self.config.short_name} ) @@ -766,9 +764,9 @@ def before_remove_contributor_message(self, node, removed): """ if self.has_auth and self.user_settings.owner == removed: return ( - u'The {addon} add-on for this {category} is authenticated by {name}. ' - u'Removing this user will also remove write access to {addon} ' - u'unless another contributor re-authenticates the add-on.' + 'The {addon} add-on for this {category} is authenticated by {name}. ' + 'Removing this user will also remove write access to {addon} ' + 'unless another contributor re-authenticates the add-on.' ).format( addon=self.config.full_name, category=node.project_or_component, @@ -789,8 +787,8 @@ def after_remove_contributor(self, node, removed, auth=None): self.user_settings.save() self.clear_auth() message = ( - u'Because the {addon} add-on for {category} "{title}" was authenticated ' - u'by {user}, authentication information has been deleted.' + 'Because the {addon} add-on for {category} "{title}" was authenticated ' + 'by {user}, authentication information has been deleted.' ).format( addon=self.config.full_name, category=markupsafe.escape(node.category_display), @@ -801,7 +799,7 @@ def after_remove_contributor(self, node, removed, auth=None): if not auth or auth.user != removed: url = node.web_url_for('node_addons') message += ( - u' You can re-authenticate on the add-ons page.' + ' You can re-authenticate on the add-ons page.' ).format(url=url) # return message @@ -812,7 +810,7 @@ def after_fork(self, node, fork, user, save=True): :return: the cloned settings """ - clone = super(BaseOAuthNodeSettings, self).after_fork( + clone = super().after_fork( node=node, fork=fork, user=user, @@ -838,9 +836,9 @@ def before_register_message(self, node, user): """ if self.has_auth: return ( - u'The contents of {addon} add-ons cannot be registered at this time; ' - u'the {addon} add-on linked to this {category} will not be included ' - u'as part of this registration.' + 'The contents of {addon} add-ons cannot be registered at this time; ' + 'the {addon} add-on linked to this {category} will not be included ' + 'as part of this registration.' ).format( addon=self.config.full_name, category=node.project_or_component, @@ -930,13 +928,13 @@ def set_auth(self, *args, **kwargs): self.list_id = None self.save() - return super(BaseCitationsNodeSettings, self).set_auth(*args, **kwargs) + return super().set_auth(*args, **kwargs) def deauthorize(self, auth=None, add_log=True): """Remove user authorization from this node and log the event.""" if add_log: self.owner.add_log( - '{0}_node_deauthorized'.format(self.provider_name), + f'{self.provider_name}_node_deauthorized', params={ 'project': self.owner.parent_id, 'node': self.owner._id, diff --git a/addons/base/serializer.py b/addons/base/serializer.py index 6187408df64..e10fc5766d2 100644 --- a/addons/base/serializer.py +++ b/addons/base/serializer.py @@ -4,7 +4,7 @@ from website.util import api_url_for, web_url_for -class AddonSerializer(object): +class AddonSerializer: __metaclass__ = abc.ABCMeta # TODO take addon_node_settings, addon_user_settings @@ -12,23 +12,28 @@ def __init__(self, node_settings=None, user_settings=None): self.node_settings = node_settings self.user_settings = user_settings - @abc.abstractproperty + @property + @abc.abstractmethod def addon_short_name(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def addon_serialized_urls(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def serialized_urls(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def user_is_owner(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def credentials_owner(self): pass @@ -80,7 +85,7 @@ def serialized_urls(self): ret = self.addon_serialized_urls # Make sure developer returns set of needed urls for url in self.REQUIRED_URLS: - msg = "addon_serialized_urls must include key '{0}'".format(url) + msg = f"addon_serialized_urls must include key '{url}'" assert url in ret, msg ret.update({'settings': web_url_for('user_addons')}) return ret @@ -94,7 +99,7 @@ def serialized_accounts(self): @property def serialized_user_settings(self): - retval = super(OAuthAddonSerializer, self).serialized_user_settings + retval = super().serialized_user_settings retval['accounts'] = [] if self.user_settings: retval['accounts'] = self.serialized_accounts @@ -202,12 +207,12 @@ def serialized_urls(self): if external_account and external_account.profile_url: ret['owner'] = external_account.profile_url - ret.update(super(CitationsAddonSerializer, self).serialized_urls) + ret.update(super().serialized_urls) return ret @property def serialized_node_settings(self): - result = super(CitationsAddonSerializer, self).serialized_node_settings + result = super().serialized_node_settings result['folder'] = { 'name': self.node_settings.fetch_folder_name } @@ -225,7 +230,7 @@ def serialize_folder(self, folder): 'id': folder['id'], 'urls': { 'fetch': self.node_settings.owner.api_url_for( - '{0}_citation_list'.format(self.addon_short_name), + f'{self.addon_short_name}_citation_list', list_id=folder['id'] ), }, @@ -235,11 +240,11 @@ def serialize_folder(self, folder): def addon_serialized_urls(self): node = self.node_settings.owner return { - 'importAuth': node.api_url_for('{0}_import_auth'.format(self.addon_short_name)), - 'folders': node.api_url_for('{0}_citation_list'.format(self.addon_short_name)), - 'config': node.api_url_for('{0}_set_config'.format(self.addon_short_name)), - 'deauthorize': node.api_url_for('{0}_deauthorize_node'.format(self.addon_short_name)), - 'accounts': node.api_url_for('{0}_account_list'.format(self.addon_short_name)), + 'importAuth': node.api_url_for(f'{self.addon_short_name}_import_auth'), + 'folders': node.api_url_for(f'{self.addon_short_name}_citation_list'), + 'config': node.api_url_for(f'{self.addon_short_name}_set_config'), + 'deauthorize': node.api_url_for(f'{self.addon_short_name}_deauthorize_node'), + 'accounts': node.api_url_for(f'{self.addon_short_name}_account_list'), } def serialize_citation(self, citation): diff --git a/addons/base/tests/base.py b/addons/base/tests/base.py index 86939652292..406cbe0a8a5 100644 --- a/addons/base/tests/base.py +++ b/addons/base/tests/base.py @@ -4,7 +4,7 @@ from osf_tests.factories import AuthUserFactory, ProjectFactory -class AddonTestCase(object): +class AddonTestCase: """General Addon TestCase that automatically sets up a user and node with an addon. @@ -31,7 +31,7 @@ class AddonTestCase(object): NODE_USER_FIELD = 'user_settings' def __init__(self, *args, **kwargs): - super(AddonTestCase,self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.node_settings = None self.project = None self.user = None @@ -57,7 +57,7 @@ def create_user_settings(self): if 'user' not in self.OWNERS: return self.user.add_addon(self.ADDON_SHORT_NAME) - assert self.user.has_addon(self.ADDON_SHORT_NAME), '{0} is not enabled'.format(self.ADDON_SHORT_NAME) + assert self.user.has_addon(self.ADDON_SHORT_NAME), f'{self.ADDON_SHORT_NAME} is not enabled' self.user_settings = self.user.get_addon(self.ADDON_SHORT_NAME) self.set_user_settings(self.user_settings) self.user_settings.save() @@ -78,7 +78,7 @@ def create_node_settings(self): def setUp(self): - super(AddonTestCase, self).setUp() + super().setUp() self.user = self.create_user() if not self.ADDON_SHORT_NAME: @@ -90,14 +90,14 @@ def setUp(self): self.create_node_settings() -class OAuthAddonTestCaseMixin(object): +class OAuthAddonTestCaseMixin: @property def ExternalAccountFactory(self): raise NotImplementedError() def __init__(self, *args, **kwargs): - super(OAuthAddonTestCaseMixin, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.auth = None self.external_account = None self.project = None diff --git a/addons/base/tests/logger.py b/addons/base/tests/logger.py index d7075322d44..9ec65021b1a 100644 --- a/addons/base/tests/logger.py +++ b/addons/base/tests/logger.py @@ -1,24 +1,24 @@ import abc -from nose.tools import * # noqa (PEP8 asserts) - from framework.auth import Auth from osf_tests.factories import AuthUserFactory, ProjectFactory -class AddonNodeLoggerTestSuiteMixinBase(object): +class AddonNodeLoggerTestSuiteMixinBase: __metaclass__ = abc.ABCMeta - @abc.abstractproperty + @property + @abc.abstractmethod def addon_short_name(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def NodeLogger(self): pass def setUp(self): - super(AddonNodeLoggerTestSuiteMixinBase, self).setUp() + super().setUp() self.auth = Auth(AuthUserFactory()) self.node = ProjectFactory(creator=self.auth.user) self.path = None @@ -29,27 +29,27 @@ def setUp(self): class StorageAddonNodeLoggerTestSuiteMixin(AddonNodeLoggerTestSuiteMixinBase): def setUp(self): - super(StorageAddonNodeLoggerTestSuiteMixin, self).setUp() + super().setUp() def test_log_file_added(self): self.logger.log('file_added', save=True) last_log = self.node.logs.latest() - assert_equal(last_log.action, '{0}_{1}'.format(self.addon_short_name, 'file_added')) + assert last_log.action == '{}_{}'.format(self.addon_short_name, 'file_added') def test_log_file_removed(self): self.logger.log('file_removed', save=True) last_log = self.node.logs.latest() - assert_equal(last_log.action, '{0}_{1}'.format(self.addon_short_name, 'file_removed')) + assert last_log.action == '{}_{}'.format(self.addon_short_name, 'file_removed') def test_log_deauthorized_when_node_settings_are_deleted(self): node_settings = self.node.get_addon(self.addon_short_name) node_settings.delete(save=True) # sanity check - assert_true(node_settings.deleted) + assert node_settings.deleted self.logger.log(action='node_deauthorized', save=True) last_log = self.node.logs.latest() - assert_equal(last_log.action, '{0}_node_deauthorized'.format(self.addon_short_name)) + assert last_log.action == f'{self.addon_short_name}_node_deauthorized' diff --git a/addons/base/tests/models.py b/addons/base/tests/models.py index de5a43ff3a8..c20f834cf24 100644 --- a/addons/base/tests/models.py +++ b/addons/base/tests/models.py @@ -1,6 +1,6 @@ import abc -import mock +from unittest import mock import pytest import pytz import datetime @@ -8,9 +8,6 @@ from django.utils import timezone from framework.auth import Auth from framework.exceptions import HTTPError -from nose.tools import (assert_equal, assert_false, assert_in, assert_is, - assert_is_none, assert_not_in, assert_raises, - assert_true) from osf.utils.permissions import ADMIN from osf_tests.factories import ProjectFactory, UserFactory from tests.utils import mock_auth @@ -20,19 +17,22 @@ pytestmark = pytest.mark.django_db -class OAuthAddonModelTestSuiteMixinBase(object): +class OAuthAddonModelTestSuiteMixinBase: ___metaclass__ = abc.ABCMeta - @abc.abstractproperty + @property + @abc.abstractmethod def short_name(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def full_name(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def ExternalAccountFactory(self): pass @@ -109,19 +109,15 @@ def test_verify_oauth_access_no_metadata(self): ) self.user_settings.save() - assert_true( - self.user_settings.verify_oauth_access( + assert self.user_settings.verify_oauth_access( node=self.node, external_account=self.external_account ) - ) - assert_false( - self.user_settings.verify_oauth_access( + assert not self.user_settings.verify_oauth_access( node=self.node, external_account=self.ExternalAccountFactory() ) - ) def test_verify_oauth_access_metadata(self): self.user_settings.grant_oauth_access( @@ -131,25 +127,21 @@ def test_verify_oauth_access_metadata(self): ) self.user_settings.save() - assert_true( - self.user_settings.verify_oauth_access( + assert self.user_settings.verify_oauth_access( node=self.node, external_account=self.external_account, metadata={'folder': 'fake_folder_id'} ) - ) - assert_false( - self.user_settings.verify_oauth_access( + assert not self.user_settings.verify_oauth_access( node=self.node, external_account=self.external_account, metadata={'folder': 'another_folder_id'} ) - ) class OAuthAddonNodeSettingsTestSuiteMixin(OAuthAddonModelTestSuiteMixinBase): - @pytest.yield_fixture(autouse=True) + @pytest.fixture(autouse=True) def _request_context(self, app): context = app.test_request_context(headers={ 'Remote-Addr': '146.9.219.56', @@ -159,15 +151,18 @@ def _request_context(self, app): yield context context.pop() - @abc.abstractproperty + @property + @abc.abstractmethod def NodeSettingsFactory(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def NodeSettingsClass(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def UserSettingsFactory(self): pass @@ -179,7 +174,7 @@ def _node_settings_class_kwargs(self, node, user_settings): } def setUp(self): - super(OAuthAddonNodeSettingsTestSuiteMixin, self).setUp() + super().setUp() self.node = ProjectFactory() self.user = self.node.creator self.external_account = self.ExternalAccountFactory() @@ -202,37 +197,34 @@ def setUp(self): @pytest.mark.django_db def test_configured_true(self): - assert_true(self.node_settings.has_auth) - assert_true(self.node_settings.complete) - assert_true(self.node_settings.configured) + assert self.node_settings.has_auth + assert self.node_settings.complete + assert self.node_settings.configured def test_configured_false(self): self.node_settings.clear_settings() self.node_settings.save() - assert_false(self.node_settings.configured) + assert not self.node_settings.configured def test_complete_true(self): - assert_true(self.node_settings.has_auth) - assert_true(self.node_settings.complete) + assert self.node_settings.has_auth + assert self.node_settings.complete def test_complete_has_auth_not_verified(self): with mock_auth(self.user): self.user_settings.revoke_oauth_access(self.external_account) self.node_settings.reload() - assert_false(self.node_settings.has_auth) - assert_false(self.node_settings.complete) - assert_equal( - self.user_settings.oauth_grants, - {self.node._id: {}} - ) + assert not self.node_settings.has_auth + assert not self.node_settings.complete + assert self.user_settings.oauth_grants == {self.node._id: {}} def test_revoke_remote_access_called(self): with mock.patch.object(self.user_settings, 'revoke_remote_oauth_access') as mock_revoke: with mock_auth(self.user): self.user_settings.revoke_oauth_access(self.external_account) - assert_equal(mock_revoke.call_count, 1) + assert mock_revoke.call_count == 1 def test_revoke_remote_access_not_called(self): user2 = UserFactory() @@ -241,26 +233,26 @@ def test_revoke_remote_access_not_called(self): with mock.patch.object(self.user_settings, 'revoke_remote_oauth_access') as mock_revoke: with mock_auth(self.user): self.user_settings.revoke_oauth_access(self.external_account) - assert_equal(mock_revoke.call_count, 0) + assert mock_revoke.call_count == 0 def test_complete_auth_false(self): self.node_settings.user_settings = None - assert_false(self.node_settings.has_auth) - assert_false(self.node_settings.complete) + assert not self.node_settings.has_auth + assert not self.node_settings.complete def test_fields(self): node_settings = self.NodeSettingsClass(owner=ProjectFactory(), user_settings=self.user_settings) node_settings.save() - assert_true(node_settings.user_settings) - assert_equal(node_settings.user_settings.owner, self.user) - assert_true(hasattr(node_settings, 'folder_id')) - assert_true(hasattr(node_settings, 'user_settings')) + assert node_settings.user_settings + assert node_settings.user_settings.owner == self.user + assert hasattr(node_settings, 'folder_id') + assert hasattr(node_settings, 'user_settings') def test_folder_defaults_to_none(self): node_settings = self.NodeSettingsClass(user_settings=self.user_settings) node_settings.save() - assert_is_none(node_settings.folder_id) + assert node_settings.folder_id is None def test_has_auth(self): self.user.external_accounts.clear() @@ -268,12 +260,12 @@ def test_has_auth(self): node = ProjectFactory() settings = self.NodeSettingsClass(user_settings=self.user_settings, owner=node) settings.save() - assert_false(settings.has_auth) + assert not settings.has_auth self.user.external_accounts.add(self.external_account) settings.set_auth(self.external_account, self.user) settings.reload() - assert_true(settings.has_auth) + assert settings.has_auth def test_clear_auth(self): node_settings = self.NodeSettingsFactory() @@ -283,8 +275,8 @@ def test_clear_auth(self): node_settings.clear_auth() - assert_is_none(node_settings.external_account) - assert_is_none(node_settings.user_settings) + assert node_settings.external_account is None + assert node_settings.user_settings is None def test_clear_settings(self): node_settings = self.NodeSettingsFactory() @@ -293,27 +285,27 @@ def test_clear_settings(self): node_settings.save() node_settings.clear_settings() - assert_is_none(node_settings.folder_id) + assert node_settings.folder_id is None def test_to_json(self): settings = self.node_settings user = UserFactory() result = settings.to_json(user) - assert_equal(result['addon_short_name'], self.short_name) + assert result['addon_short_name'] == self.short_name def test_delete(self): - assert_true(self.node_settings.user_settings) - assert_true(self.node_settings.folder_id) + assert self.node_settings.user_settings + assert self.node_settings.folder_id old_logs = list(self.node.logs.all()) mock_now = datetime.datetime(2017, 3, 16, 11, 00, tzinfo=pytz.utc) with mock.patch.object(timezone, 'now', return_value=mock_now): self.node_settings.delete() self.node_settings.save() - assert_is(self.node_settings.user_settings, None) - assert_is(self.node_settings.folder_id, None) - assert_true(self.node_settings.is_deleted) - assert_equal(self.node_settings.deleted, mock_now) - assert_equal(list(self.node.logs.all()), list(old_logs)) + assert self.node_settings.user_settings is None + assert self.node_settings.folder_id is None + assert self.node_settings.is_deleted + assert self.node_settings.deleted == mock_now + assert list(self.node.logs.all()) == list(old_logs) def test_on_delete(self): self.user.delete_addon( @@ -322,32 +314,32 @@ def test_on_delete(self): self.node_settings.reload() - assert_is_none(self.node_settings.external_account) - assert_is_none(self.node_settings.user_settings) + assert self.node_settings.external_account is None + assert self.node_settings.user_settings is None def test_deauthorize(self): - assert_true(self.node_settings.user_settings) - assert_true(self.node_settings.folder_id) + assert self.node_settings.user_settings + assert self.node_settings.folder_id self.node_settings.deauthorize(auth=Auth(self.user)) self.node_settings.save() - assert_is(self.node_settings.user_settings, None) - assert_is(self.node_settings.folder_id, None) + assert self.node_settings.user_settings is None + assert self.node_settings.folder_id is None last_log = self.node.logs.first() - assert_equal(last_log.action, '{0}_node_deauthorized'.format(self.short_name)) + assert last_log.action == f'{self.short_name}_node_deauthorized' params = last_log.params - assert_in('node', params) - assert_in('project', params) + assert 'node' in params + assert 'project' in params def test_set_folder(self): folder_id = '1234567890' self.node_settings.set_folder(folder_id, auth=Auth(self.user)) self.node_settings.save() # Folder was set - assert_equal(self.node_settings.folder_id, folder_id) + assert self.node_settings.folder_id == folder_id # Log was saved last_log = self.node.logs.first() - assert_equal(last_log.action, '{0}_folder_selected'.format(self.short_name)) + assert last_log.action == f'{self.short_name}_folder_selected' def test_set_user_auth(self): node_settings = self.NodeSettingsFactory() @@ -361,14 +353,14 @@ def test_set_user_auth(self): node_settings.set_auth(external_account, user_settings.owner) node_settings.save() - assert_true(node_settings.has_auth) - assert_equal(node_settings.user_settings, user_settings) + assert node_settings.has_auth + assert node_settings.user_settings == user_settings # A log was saved last_log = node_settings.owner.logs.first() - assert_equal(last_log.action, '{0}_node_authorized'.format(self.short_name)) + assert last_log.action == f'{self.short_name}_node_authorized' log_params = last_log.params - assert_equal(log_params['node'], node_settings.owner._id) - assert_equal(last_log.user, user_settings.owner) + assert log_params['node'] == node_settings.owner._id + assert last_log.user == user_settings.owner def test_serialize_credentials(self): self.user_settings.external_accounts[0].oauth_key = 'key-11' @@ -376,23 +368,23 @@ def test_serialize_credentials(self): credentials = self.node_settings.serialize_waterbutler_credentials() expected = {'token': self.node_settings.external_account.oauth_key} - assert_equal(credentials, expected) + assert credentials == expected def test_serialize_credentials_not_authorized(self): self.node_settings.user_settings = None self.node_settings.save() - with assert_raises(exceptions.AddonError): + with pytest.raises(exceptions.AddonError): self.node_settings.serialize_waterbutler_credentials() def test_serialize_settings(self): settings = self.node_settings.serialize_waterbutler_settings() expected = {'folder': self.node_settings.folder_id} - assert_equal(settings, expected) + assert settings == expected def test_serialize_settings_not_configured(self): self.node_settings.clear_settings() self.node_settings.save() - with assert_raises(exceptions.AddonError): + with pytest.raises(exceptions.AddonError): self.node_settings.serialize_waterbutler_settings() def test_create_log(self): @@ -405,22 +397,16 @@ def test_create_log(self): metadata={'path': path, 'materialized': path}, ) self.node.reload() - assert_equal(self.node.logs.count(), nlog + 1) - assert_equal( - self.node.logs.latest().action, - '{0}_{1}'.format(self.short_name, action), - ) - assert_equal( - self.node.logs.latest().params['path'], - path - ) + assert self.node.logs.count() == nlog + 1 + assert self.node.logs.latest().action == f'{self.short_name}_{action}' + assert self.node.logs.latest().params['path'] == path def test_after_fork_by_authorized_user(self): fork = ProjectFactory() clone = self.node_settings.after_fork( node=self.node, fork=fork, user=self.user_settings.owner ) - assert_equal(clone.user_settings, self.user_settings) + assert clone.user_settings == self.user_settings def test_after_fork_by_unauthorized_user(self): fork = ProjectFactory() @@ -429,46 +415,48 @@ def test_after_fork_by_unauthorized_user(self): node=self.node, fork=fork, user=user, save=True ) - assert_is(clone.user_settings, None) + assert clone.user_settings is None def test_before_remove_contributor_message(self): message = self.node_settings.before_remove_contributor( self.node, self.user) - assert_true(message) - assert_in(self.user.fullname, message) - assert_in(self.node.project_or_component, message) + assert message + assert self.user.fullname in message + assert self.node.project_or_component in message def test_after_remove_authorized_user_not_self(self): message = self.node_settings.after_remove_contributor( self.node, self.user_settings.owner) self.node_settings.save() - assert_is_none(self.node_settings.user_settings) - assert_true(message) - assert_in('You can re-authenticate', message) + assert self.node_settings.user_settings is None + assert message + assert 'You can re-authenticate' in message def test_after_remove_authorized_user_self(self): auth = Auth(user=self.user_settings.owner) message = self.node_settings.after_remove_contributor( self.node, self.user_settings.owner, auth) self.node_settings.save() - assert_is_none(self.node_settings.user_settings) - assert_true(message) - assert_not_in('You can re-authenticate', message) + assert self.node_settings.user_settings is None + assert message + assert 'You can re-authenticate' not in message def test_after_delete(self): self.node.remove_node(Auth(user=self.node.creator)) # Ensure that changes to node settings have been saved self.node_settings.reload() - assert_is_none(self.node_settings.user_settings) - assert_is_none(self.node_settings.folder_id) + assert self.node_settings.user_settings is None + assert self.node_settings.folder_id is None class OAuthCitationsTestSuiteMixinBase(OAuthAddonModelTestSuiteMixinBase): - @abc.abstractproperty + @property + @abc.abstractmethod def ProviderClass(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def OAuthProviderClass(self): pass @@ -478,7 +466,7 @@ class OAuthCitationsNodeSettingsTestSuiteMixin( OAuthCitationsTestSuiteMixinBase): def setUp(self): - super(OAuthCitationsNodeSettingsTestSuiteMixin, self).setUp() + super().setUp() self.user_settings.grant_oauth_access( node=self.node, external_account=self.external_account, @@ -489,18 +477,12 @@ def setUp(self): def test_fetch_folder_name_root(self): self.node_settings.list_id = 'ROOT' - assert_equal( - self.node_settings.fetch_folder_name, - 'All Documents' - ) + assert self.node_settings.fetch_folder_name == 'All Documents' def test_selected_folder_name_empty(self): self.node_settings.list_id = None - assert_equal( - self.node_settings.fetch_folder_name, - '' - ) + assert self.node_settings.fetch_folder_name == '' def test_selected_folder_name(self): # Mock the return from api call to get the folder's name @@ -510,25 +492,22 @@ def test_selected_folder_name(self): with mock.patch.object(self.OAuthProviderClass, '_folder_metadata', return_value=mock_folder): name = self.node_settings.fetch_folder_name - assert_equal( - name, - 'Fake Folder' - ) + assert name == 'Fake Folder' def test_api_not_cached(self): # The first call to .api returns a new object with mock.patch.object(self.NodeSettingsClass, 'oauth_provider') as mock_api: api = self.node_settings.api mock_api.assert_called_once_with(account=self.external_account) - assert_equal(api, mock_api()) + assert api == mock_api() def test_api_cached(self): # Repeated calls to .api returns the same object with mock.patch.object(self.NodeSettingsClass, 'oauth_provider') as mock_api: self.node_settings._api = 'testapi' api = self.node_settings.api - assert_false(mock_api.called) - assert_equal(api, 'testapi') + assert not mock_api.called + assert api == 'testapi' ############# Overrides ############## # `pass` due to lack of waterbutler- # @@ -563,7 +542,7 @@ def test_set_folder(self): self.node_settings.clear_settings() self.node_settings.save() - assert_is_none(self.node_settings.list_id) + assert self.node_settings.list_id is None provider = self.ProviderClass() @@ -576,25 +555,20 @@ def test_set_folder(self): ) # instance was updated - assert_equal( - self.node_settings.list_id, - 'fake-folder-id', - ) + assert self.node_settings.list_id == 'fake-folder-id' # user_settings was updated # TODO: the call to grant_oauth_access should be mocked - assert_true( - self.user_settings.verify_oauth_access( + assert self.user_settings.verify_oauth_access( node=self.node, external_account=self.external_account, metadata={'folder': 'fake-folder-id'} ) - ) log = self.node.logs.latest() - assert_equal(log.action, '{}_folder_selected'.format(self.short_name)) - assert_equal(log.params['folder_id'], folder_id) - assert_equal(log.params['folder_name'], folder_name) + assert log.action == f'{self.short_name}_folder_selected' + assert log.params['folder_id'] == folder_id + assert log.params['folder_name'] == folder_name @mock.patch('framework.status.push_status_message') def test_remove_contributor_authorizer(self, mock_push_status): @@ -603,24 +577,24 @@ def test_remove_contributor_authorizer(self, mock_push_status): self.node.remove_contributor(self.node.creator, auth=Auth(user=contributor)) self.node_settings.reload() self.user_settings.reload() - assert_false(self.node_settings.has_auth) - assert_false(self.user_settings.verify_oauth_access(self.node, self.external_account)) + assert not self.node_settings.has_auth + assert not self.user_settings.verify_oauth_access(self.node, self.external_account) def test_remove_contributor_not_authorizer(self): contributor = UserFactory() self.node.add_contributor(contributor) self.node.remove_contributor(contributor, auth=Auth(user=self.node.creator)) - assert_true(self.node_settings.has_auth) - assert_true(self.user_settings.verify_oauth_access(self.node, self.external_account)) + assert self.node_settings.has_auth + assert self.user_settings.verify_oauth_access(self.node, self.external_account) @mock.patch('framework.status.push_status_message') def test_fork_by_authorizer(self, mock_push_status): fork = self.node.fork_node(auth=Auth(user=self.node.creator)) self.user_settings.reload() - assert_true(fork.get_addon(self.short_name).has_auth) - assert_true(self.user_settings.verify_oauth_access(fork, self.external_account)) + assert fork.get_addon(self.short_name).has_auth + assert self.user_settings.verify_oauth_access(fork, self.external_account) @mock.patch('framework.status.push_status_message') def test_fork_not_by_authorizer(self, mock_push_status): @@ -628,17 +602,18 @@ def test_fork_not_by_authorizer(self, mock_push_status): self.node.add_contributor(contributor) fork = self.node.fork_node(auth=Auth(user=contributor)) - assert_false(fork.get_addon(self.short_name).has_auth) - assert_false(self.user_settings.verify_oauth_access(fork, self.external_account)) + assert not fork.get_addon(self.short_name).has_auth + assert not self.user_settings.verify_oauth_access(fork, self.external_account) class CitationAddonProviderTestSuiteMixin(OAuthCitationsTestSuiteMixinBase): - @abc.abstractproperty + @property + @abc.abstractmethod def ApiExceptionClass(self): pass def setUp(self): - super(CitationAddonProviderTestSuiteMixin, self).setUp() + super().setUp() self.provider = self.OAuthProviderClass() @abc.abstractmethod @@ -656,8 +631,8 @@ def test_citation_lists(self): mock_account = mock.Mock() self.provider.account = mock_account res = self.provider.citation_lists(self.ProviderClass()._extract_folder) - assert_equal(res[1]['name'], mock_folders[0].name) - assert_equal(res[1]['id'], mock_folders[0].json['id']) + assert res[1]['name'] == mock_folders[0].name + assert res[1]['id'] == mock_folders[0].json['id'] def test_client_not_cached(self): # The first call to .client returns a new client @@ -667,15 +642,15 @@ def test_client_not_cached(self): self.provider.account = mock_account self.provider.client mock_get_client.assert_called_with() - assert_true(mock_get_client.called) + assert mock_get_client.called def test_client_cached(self): # Repeated calls to .client returns the same client with mock.patch.object(self.OAuthProviderClass, '_get_client') as mock_get_client: self.provider._client = mock.Mock() res = self.provider.client - assert_equal(res, self.provider._client) - assert_false(mock_get_client.called) + assert res == self.provider._client + assert not mock_get_client.called def test_has_access(self): with mock.patch.object(self.OAuthProviderClass, '_get_client') as mock_get_client: @@ -686,6 +661,6 @@ def test_has_access(self): mock_client.folders.list.side_effect = self.ApiExceptionClass(mock_error) mock_client.collections.side_effect = self.ApiExceptionClass(mock_error) mock_get_client.return_value = mock_client - with assert_raises(HTTPError) as exc_info: + with pytest.raises(HTTPError) as exc_info: self.provider.client - assert_equal(exc_info.exception.code, 403) + assert exc_info.value.code == 403 diff --git a/addons/base/tests/serializers.py b/addons/base/tests/serializers.py index 9c331bca343..545761dcf12 100644 --- a/addons/base/tests/serializers.py +++ b/addons/base/tests/serializers.py @@ -1,23 +1,25 @@ import abc -import mock +from unittest import mock +import pytest + from framework.auth import Auth -from nose.tools import (assert_equal, assert_false, assert_in, - assert_is_not_none, assert_raises, assert_true) from osf_tests.factories import ProjectFactory, AuthUserFactory from tests.utils import mock_auth from website.util import web_url_for -class AddonSerializerTestSuiteMixin(object): +class AddonSerializerTestSuiteMixin: __metaclass__ = abc.ABCMeta - @abc.abstractproperty + @property + @abc.abstractmethod def Serializer(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def ExternalAccountFactory(self): pass @@ -29,30 +31,27 @@ def set_user_settings(self, user): def set_node_settings(self, user_settings): pass - @abc.abstractproperty + @property + @abc.abstractmethod def required_settings(self): pass - @abc.abstractproperty + @property + @abc.abstractmethod def required_settings_authorized(self): pass def setUp(self): - super(AddonSerializerTestSuiteMixin, self).setUp() + super().setUp() self.user = AuthUserFactory() self.node = ProjectFactory(creator=self.user) self.set_user_settings(self.user) - assert_is_not_none( - getattr(self, 'user_settings'), - "'set_user_settings' should set the 'user_settings' attribute of the instance to an instance of \ + assert getattr(self, 'user_settings') is not None, "'set_user_settings' should set the 'user_settings' attribute of the instance to an instance of \ the appropriate user settings model." - ) + self.set_node_settings(self.user_settings) - assert_is_not_none( - getattr(self, 'node_settings'), - "'set_node_settings' should set the 'user_settings' attribute of the instance to an instance of \ + assert getattr(self, 'node_settings') is not None, "'set_node_settings' should set the 'user_settings' attribute of the instance to an instance of \ the appropriate node settings model." - ) self.ser = self.Serializer( user_settings=self.user_settings, @@ -63,15 +62,15 @@ def test_serialized_node_settings_unauthorized(self): with mock.patch.object(type(self.node_settings), 'has_auth', return_value=False): serialized = self.ser.serialized_node_settings for setting in self.required_settings: - assert_in(setting, serialized) + assert setting in serialized def test_serialized_node_settings_authorized(self): with mock.patch.object(type(self.node_settings), 'has_auth', return_value=True): serialized = self.ser.serialized_node_settings for setting in self.required_settings: - assert_in(setting, serialized) + assert setting in serialized for setting in self.required_settings_authorized: - assert_in(setting, serialized) + assert setting in serialized class OAuthAddonSerializerTestSuiteMixin(AddonSerializerTestSuiteMixin): @@ -88,38 +87,38 @@ def set_node_settings(self, user_settings): def test_credentials_owner(self): owner = self.ser.credentials_owner - assert_equal(owner._id, self.user_settings.owner._id) + assert owner._id == self.user_settings.owner._id def test_user_is_owner_no_user_settings(self): ser = self.Serializer(node_settings=self.node_settings) - assert_false(ser.user_is_owner) + assert not ser.user_is_owner def test_user_is_owner_no_node_settings(self): ser = self.Serializer(user_settings=self.user_settings) - assert_false(ser.user_is_owner) + assert not ser.user_is_owner def test_user_is_owner_node_not_authorized_user_has_no_accounts(self): self.user.external_accounts.clear() - assert_false(self.user_settings.external_accounts.count()) - assert_false(self.ser.user_is_owner) + assert not self.user_settings.external_accounts.count() + assert not self.ser.user_is_owner def test_user_is_owner_node_not_authorized_user_has_accounts(self): - assert_true(self.user_settings.external_accounts.count()) - assert_true(self.ser.user_is_owner) + assert self.user_settings.external_accounts.count() + assert self.ser.user_is_owner def test_user_is_owner_node_authorized_user_is_not_owner(self): self.node_settings.external_account = self.ExternalAccountFactory() with mock.patch('addons.base.models.BaseOAuthUserSettings.verify_oauth_access', return_value=True): self.user.external_accounts.clear() - assert_false(self.ser.user_is_owner) + assert not self.ser.user_is_owner def test_user_is_owner_node_authorized_user_is_owner(self): - assert_true(self.ser.user_is_owner) + assert self.ser.user_is_owner def test_serialized_urls_checks_required(self): with mock.patch.object(self.ser, 'REQUIRED_URLS', ('foobar', )): - with assert_raises(AssertionError): + with pytest.raises(AssertionError): self.ser.serialized_urls def test_serialized_acccounts(self): @@ -129,8 +128,8 @@ def test_serialized_acccounts(self): with mock.patch.object(type(self.ser), 'serialize_account') as mock_serialize_account: mock_serialize_account.return_value = {} serialized = self.ser.serialized_accounts - assert_equal(len(serialized), self.user.external_accounts.count()) - assert_equal(mock_serialize_account.call_count, len(serialized)) + assert len(serialized) == self.user.external_accounts.count() + assert mock_serialize_account.call_count == len(serialized) def test_serialize_acccount(self): ea = self.ExternalAccountFactory() @@ -143,22 +142,22 @@ def test_serialize_acccount(self): 'profile_url': ea.profile_url, 'nodes': [], } - assert_equal(self.ser.serialize_account(ea), expected) + assert self.ser.serialize_account(ea) == expected def test_serialized_user_settings(self): with mock.patch.object(self.Serializer, 'serialized_accounts', return_value=[]): serialized = self.ser.serialized_user_settings - assert_in('accounts', serialized) + assert 'accounts' in serialized def test_serialize_granted_node(self): with mock_auth(self.user): serialized = self.ser.serialize_granted_node(self.node, auth=Auth(self.user)) for key in ('id', 'title', 'urls'): - assert_in(key, serialized) - assert_equal(self.node._id, serialized['id']) - assert_equal(self.node.title, serialized['title']) - assert_in('view', serialized['urls']) - assert_equal(serialized['urls']['view'], self.node.url) + assert key in serialized + assert self.node._id == serialized['id'] + assert self.node.title == serialized['title'] + assert 'view' in serialized['urls'] + assert serialized['urls']['view'] == self.node.url class StorageAddonSerializerTestSuiteMixin(OAuthAddonSerializerTestSuiteMixin): @@ -166,7 +165,8 @@ class StorageAddonSerializerTestSuiteMixin(OAuthAddonSerializerTestSuiteMixin): required_settings = ('userIsOwner', 'nodeHasAuth', 'urls', 'userHasAuth') required_settings_authorized = ('ownerName', ) - @abc.abstractproperty + @property + @abc.abstractmethod def client(self): """Provide a mocked version of this provider's client (i.e. the client should not make acutal API calls). @@ -181,27 +181,27 @@ def test_serialize_settings_unauthorized(self): with mock.patch.object(type(self.node_settings), 'has_auth', return_value=False): serialized = self.ser.serialize_settings(self.node_settings, self.user, self.client) for key in self.required_settings: - assert_in(key, serialized) + assert key in serialized def test_serialize_settings_authorized(self): with mock.patch.object(type(self.node_settings), 'has_auth', return_value=True): serialized = self.ser.serialize_settings(self.node_settings, self.user, self.client) for key in self.required_settings: - assert_in(key, serialized) - assert_in('owner', serialized['urls']) - assert_equal(serialized['urls']['owner'], web_url_for( + assert key in serialized + assert 'owner' in serialized['urls'] + assert serialized['urls']['owner'] == web_url_for( 'profile_view_id', uid=self.user_settings.owner._id - )) - assert_in('ownerName', serialized) - assert_equal(serialized['ownerName'], self.user_settings.owner.fullname) - assert_in('folder', serialized) + ) + assert 'ownerName' in serialized + assert serialized['ownerName'] == self.user_settings.owner.fullname + assert 'folder' in serialized def test_serialize_settings_authorized_no_folder(self): with mock.patch.object(type(self.node_settings), 'has_auth', return_value=True): serialized = self.ser.serialize_settings(self.node_settings, self.user, self.client) - assert_in('folder', serialized) - assert_equal(serialized['folder'], {'name': None, 'path': None}) + assert 'folder' in serialized + assert serialized['folder'] == {'name': None, 'path': None} def test_serialize_settings_authorized_folder_is_set(self): self.set_provider_id('foo') @@ -209,26 +209,27 @@ def test_serialize_settings_authorized_folder_is_set(self): with mock.patch.object(self.ser, 'serialized_folder') as mock_serialized_folder: mock_serialized_folder.return_value = {} serialized = self.ser.serialize_settings(self.node_settings, self.user, self.client) - assert_in('folder', serialized) - assert_true(mock_serialized_folder.called) + assert 'folder' in serialized + assert mock_serialized_folder.called class CitationAddonSerializerTestSuiteMixin(OAuthAddonSerializerTestSuiteMixin): required_settings = ('userIsOwner', 'nodeHasAuth', 'urls', 'userHasAuth') required_settings_authorized = ('ownerName', ) - @abc.abstractproperty + @property + @abc.abstractmethod def folder(self): pass def test_serialize_folder(self): serialized_folder = self.ser.serialize_folder(self.folder) - assert_equal(serialized_folder['id'], self.folder['id']) - assert_equal(serialized_folder['name'], self.folder.name) - assert_equal(serialized_folder['kind'], 'folder') + assert serialized_folder['id'] == self.folder['id'] + assert serialized_folder['name'] == self.folder.name + assert serialized_folder['kind'] == 'folder' def test_serialize_citation(self): serialized_citation = self.ser.serialize_citation(self.folder) - assert_equal(serialized_citation['csl'], self.folder) - assert_equal(serialized_citation['id'], self.folder['id']) - assert_equal(serialized_citation['kind'], 'file') + assert serialized_citation['csl'] == self.folder + assert serialized_citation['id'] == self.folder['id'] + assert serialized_citation['kind'] == 'file' diff --git a/addons/base/tests/utils.py b/addons/base/tests/utils.py index 432371077f6..5ab0bbbf45c 100644 --- a/addons/base/tests/utils.py +++ b/addons/base/tests/utils.py @@ -7,7 +7,7 @@ from website.settings import MFR_SERVER_URL -class MockFolder(dict, object): +class MockFolder(dict): def __init__(self): self.name = 'Fake Folder' @@ -18,7 +18,7 @@ def __init__(self): self['id'] = 'Fake Key' -class MockLibrary(dict, object): +class MockLibrary(dict): def __init__(self): self.name = 'Fake Library' diff --git a/addons/base/tests/views.py b/addons/base/tests/views.py index 290b4571f81..33675736754 100644 --- a/addons/base/tests/views.py +++ b/addons/base/tests/views.py @@ -1,6 +1,6 @@ -from future.moves.urllib.parse import urlparse, parse_qs -import mock -from nose.tools import * # noqa +import pytest +from urllib.parse import urlparse, parse_qs +from unittest import mock import responses from rest_framework import status as http_status from waffle.testutils import override_flag @@ -58,9 +58,9 @@ def test_oauth_finish(self): with mock.patch.object(self.Provider, 'auth_callback') as mock_callback: mock_callback.return_value = True res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK name, args, kwargs = mock_callback.mock_calls[0] - assert_equal(kwargs['user']._id, self.user._id) + assert kwargs['user']._id == self.user._id @mock.patch('website.oauth.views.requests.get') def test_oauth_finish_enable_gv(self, mock_requests_get): @@ -87,11 +87,11 @@ def test_delete_external_account(self): external_account_id=self.external_account._id ) res = self.app.delete(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.user.reload() for account in self.user.external_accounts.all(): - assert_not_equal(account._id, self.external_account._id) - assert_false(self.user.external_accounts.exists()) + assert account._id != self.external_account._id + assert not self.user.external_accounts.exists() def test_delete_external_account_not_owner(self): other_user = AuthUserFactory() @@ -99,14 +99,14 @@ def test_delete_external_account_not_owner(self): 'oauth_disconnect', external_account_id=self.external_account._id ) - res = self.app.delete(url, auth=other_user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + res = self.app.delete(url, auth=other_user.auth) + assert res.status_code == http_status.HTTP_403_FORBIDDEN class OAuthAddonConfigViewsTestCaseMixin(OAuthAddonTestCaseMixin): def __init__(self, *args, **kwargs): - super(OAuthAddonConfigViewsTestCaseMixin,self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.node_settings = None @property @@ -137,18 +137,18 @@ def test_import_auth(self): node = ProjectFactory(creator=self.user) node_settings = node.get_or_add_addon(self.ADDON_SHORT_NAME, auth=Auth(self.user)) node.save() - url = node.api_url_for('{0}_import_auth'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = node.api_url_for(f'{self.ADDON_SHORT_NAME}_import_auth') + res = self.app.put(url, json={ 'external_account_id': ea._id }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('result', res.json) + assert res.status_code == http_status.HTTP_200_OK + assert 'result' in res.json node_settings.reload() - assert_equal(node_settings.external_account._id, ea._id) + assert node_settings.external_account._id == ea._id node.reload() last_log = node.logs.latest() - assert_equal(last_log.action, '{0}_node_authorized'.format(self.ADDON_SHORT_NAME)) + assert last_log.action == f'{self.ADDON_SHORT_NAME}_node_authorized' def test_import_auth_invalid_account(self): ea = self.ExternalAccountFactory() @@ -156,11 +156,11 @@ def test_import_auth_invalid_account(self): node = ProjectFactory(creator=self.user) node.add_addon(self.ADDON_SHORT_NAME, auth=self.auth) node.save() - url = node.api_url_for('{0}_import_auth'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = node.api_url_for(f'{self.ADDON_SHORT_NAME}_import_auth') + res = self.app.put(url, json={ 'external_account_id': ea._id - }, auth=self.user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + }, auth=self.user.auth, ) + assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_import_auth_cant_write_node(self): ea = self.ExternalAccountFactory() @@ -173,73 +173,70 @@ def test_import_auth_cant_write_node(self): node.add_contributor(user, permissions=permissions.READ, auth=self.auth, save=True) node.add_addon(self.ADDON_SHORT_NAME, auth=self.auth) node.save() - url = node.api_url_for('{0}_import_auth'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = node.api_url_for(f'{self.ADDON_SHORT_NAME}_import_auth') + res = self.app.put(url, json={ 'external_account_id': ea._id - }, auth=user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + }, auth=user.auth, ) + assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_set_config(self): self.node_settings.set_auth(self.external_account, self.user) - url = self.project.api_url_for('{0}_set_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config') + res = self.app.put(url, json={ 'selected': self.folder }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.project.reload() - assert_equal( - self.project.logs.latest().action, - '{0}_folder_selected'.format(self.ADDON_SHORT_NAME) - ) - assert_equal(res.json['result']['folder']['path'], self.folder['path']) + assert self.project.logs.latest().action == f'{self.ADDON_SHORT_NAME}_folder_selected' + assert res.json['result']['folder']['path'] == self.folder['path'] def test_get_config(self): - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') with mock.patch.object(type(self.Serializer()), 'credentials_are_valid', return_value=True): res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('result', res.json) + assert res.status_code == http_status.HTTP_200_OK + assert 'result' in res.json serialized = self.Serializer().serialize_settings( self.node_settings, self.user, self.client ) - assert_equal(serialized, res.json['result']) + assert serialized == res.json['result'] def test_get_config_unauthorized(self): - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') user = AuthUserFactory() self.project.add_contributor(user, permissions=permissions.READ, auth=self.auth, save=True) - res = self.app.get(url, auth=user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + res = self.app.get(url, auth=user.auth, ) + assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_get_config_not_logged_in(self): - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.get(url, auth=None, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_302_FOUND) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') + res = self.app.get(url, auth=None) + assert res.status_code == http_status.HTTP_302_FOUND def test_account_list_single(self): - url = api_url_for('{0}_account_list'.format(self.ADDON_SHORT_NAME)) + url = api_url_for(f'{self.ADDON_SHORT_NAME}_account_list') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('accounts', res.json) - assert_equal(len(res.json['accounts']), 1) + assert res.status_code == http_status.HTTP_200_OK + assert 'accounts' in res.json + assert len(res.json['accounts']) == 1 def test_account_list_multiple(self): ea = self.ExternalAccountFactory() self.user.external_accounts.add(ea) self.user.save() - url = api_url_for('{0}_account_list'.format(self.ADDON_SHORT_NAME)) + url = api_url_for(f'{self.ADDON_SHORT_NAME}_account_list') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('accounts', res.json) - assert_equal(len(res.json['accounts']), 2) + assert res.status_code == http_status.HTTP_200_OK + assert 'accounts' in res.json + assert len(res.json['accounts']) == 2 def test_account_list_not_authorized(self): - url = api_url_for('{0}_account_list'.format(self.ADDON_SHORT_NAME)) - res = self.app.get(url, auth=None, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_302_FOUND) + url = api_url_for(f'{self.ADDON_SHORT_NAME}_account_list') + res = self.app.get(url, auth=None) + assert res.status_code == http_status.HTTP_302_FOUND def test_folder_list(self): # Note: if your addon's folder_list view makes API calls @@ -247,23 +244,23 @@ def test_folder_list(self): # subclass, mock any API calls, and call super. self.node_settings.set_auth(self.external_account, self.user) self.node_settings.save() - url = self.project.api_url_for('{0}_folder_list'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_folder_list') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK # TODO test result serialization? def test_deauthorize_node(self): - url = self.project.api_url_for('{0}_deauthorize_node'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_deauthorize_node') res = self.app.delete(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.node_settings.reload() - assert_is_none(self.node_settings.external_account) - assert_false(self.node_settings.has_auth) + assert self.node_settings.external_account is None + assert not self.node_settings.has_auth # A log event was saved self.project.reload() last_log = self.project.logs.latest() - assert_equal(last_log.action, '{0}_node_deauthorized'.format(self.ADDON_SHORT_NAME)) + assert last_log.action == f'{self.ADDON_SHORT_NAME}_node_deauthorized' class OAuthCitationAddonConfigViewsTestCaseMixin(OAuthAddonConfigViewsTestCaseMixin): @@ -311,7 +308,7 @@ def mockResponses(self): raise NotImplementedError() def setUp(self): - super(OAuthCitationAddonConfigViewsTestCaseMixin, self).setUp() + super().setUp() self.mock_verify = mock.patch.object( self.client, '_verify_client_validity' @@ -320,57 +317,54 @@ def setUp(self): def tearDown(self): self.mock_verify.stop() - super(OAuthCitationAddonConfigViewsTestCaseMixin, self).tearDown() + super().tearDown() def test_set_config(self): with mock.patch.object(self.client, '_folder_metadata') as mock_metadata: mock_metadata.return_value = self.folder - url = self.project.api_url_for('{0}_set_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config') + res = self.app.put(url, json={ 'external_list_id': self.folder.json['id'], 'external_list_name': self.folder.name, }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.project.reload() - assert_equal( - self.project.logs.latest().action, - '{0}_folder_selected'.format(self.ADDON_SHORT_NAME) - ) - assert_equal(res.json['result']['folder']['name'], self.folder.name) + assert self.project.logs.latest().action == f'{self.ADDON_SHORT_NAME}_folder_selected' + assert res.json['result']['folder']['name'] == self.folder.name def test_get_config(self): with mock.patch.object(self.client, '_folder_metadata') as mock_metadata: mock_metadata.return_value = self.folder self.node_settings.api._client = 'client' self.node_settings.save() - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('result', res.json) + assert res.status_code == http_status.HTTP_200_OK + assert 'result' in res.json result = res.json['result'] serialized = self.Serializer( node_settings=self.node_settings, user_settings=self.node_settings.user_settings ).serialized_node_settings serialized['validCredentials'] = self.citationsProvider().check_credentials(self.node_settings) - assert_equal(serialized, result) + assert serialized == result def test_folder_list(self): with mock.patch.object(self.client, '_get_folders'): self.node_settings.set_auth(self.external_account, self.user) self.node_settings.save() - url = self.project.api_url_for('{0}_citation_list'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_citation_list') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK def test_check_credentials(self): with mock.patch.object(self.client, 'client', new_callable=mock.PropertyMock) as mock_client: self.provider = self.citationsProvider() mock_client.side_effect = HTTPError(403) - assert_false(self.provider.check_credentials(self.node_settings)) + assert not self.provider.check_credentials(self.node_settings) mock_client.side_effect = HTTPError(402) - with assert_raises(HTTPError): + with pytest.raises(HTTPError): self.provider.check_credentials(self.node_settings) def test_widget_view_complete(self): @@ -382,25 +376,25 @@ def test_widget_view_complete(self): self.folder.name, Auth(self.user) ) - assert_true(self.node_settings.complete) - assert_equal(self.node_settings.list_id, 'Fake Key') + assert self.node_settings.complete + assert self.node_settings.list_id == 'Fake Key' res = self.citationsProvider().widget(self.project.get_addon(self.ADDON_SHORT_NAME)) - assert_true(res['complete']) - assert_equal(res['list_id'], 'Fake Key') + assert res['complete'] + assert res['list_id'] == 'Fake Key' def test_widget_view_incomplete(self): # JSON: tell the widget when it hasn't been configured self.node_settings.clear_settings() self.node_settings.save() - assert_false(self.node_settings.complete) - assert_equal(self.node_settings.list_id, None) + assert not self.node_settings.complete + assert self.node_settings.list_id is None res = self.citationsProvider().widget(self.project.get_addon(self.ADDON_SHORT_NAME)) - assert_false(res['complete']) - assert_is_none(res['list_id']) + assert not res['complete'] + assert res['list_id'] is None @responses.activate def test_citation_list_root(self): @@ -415,13 +409,13 @@ def test_citation_list_root(self): ) res = self.app.get( - self.project.api_url_for('{0}_citation_list'.format(self.ADDON_SHORT_NAME)), + self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_citation_list'), auth=self.user.auth ) root = res.json['contents'][0] - assert_equal(root['kind'], 'folder') - assert_equal(root['id'], 'ROOT') - assert_equal(root['parent_list_id'], '__') + assert root['kind'] == 'folder' + assert root['id'] == 'ROOT' + assert root['parent_list_id'] == '__' @responses.activate def test_citation_list_non_root(self): @@ -445,15 +439,15 @@ def test_citation_list_non_root(self): ) res = self.app.get( - self.project.api_url_for('{0}_citation_list'.format(self.ADDON_SHORT_NAME), list_id='ROOT'), + self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_citation_list', list_id='ROOT'), auth=self.user.auth ) children = res.json['contents'] - assert_equal(len(children), 7) - assert_equal(children[0]['kind'], 'folder') - assert_equal(children[1]['kind'], 'file') - assert_true(children[1].get('csl') is not None) + assert len(children) == 7 + assert children[0]['kind'] == 'folder' + assert children[1]['kind'] == 'file' + assert children[1].get('csl') is not None @responses.activate def test_citation_list_non_linked_or_child_non_authorizer(self): @@ -482,8 +476,7 @@ def test_citation_list_non_linked_or_child_non_authorizer(self): ) res = self.app.get( - self.project.api_url_for('{0}_citation_list'.format(self.ADDON_SHORT_NAME), list_id='ROOT'), + self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_citation_list', list_id='ROOT'), auth=non_authorizing_user.auth, - expect_errors=True ) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + assert res.status_code == http_status.HTTP_403_FORBIDDEN diff --git a/addons/base/utils.py b/addons/base/utils.py index 92be45b3739..c86f790fecd 100644 --- a/addons/base/utils.py +++ b/addons/base/utils.py @@ -10,6 +10,7 @@ def get_mfr_url(target, provider_name): return target.osfstorage_region.mfr_url return MFR_SERVER_URL + def serialize_addon_config(config, user): lookup = config.template_lookup @@ -42,13 +43,13 @@ def format_last_known_metadata(auth, node, file, error_type): parts = [ """This file was """ if last_seen or hashes or path or size else '', """last seen on {} UTC """.format(last_seen.strftime('%c')) if last_seen else '', - """and found at path {} """.format(markupsafe.escape(path)) if last_seen and path else '', - """last found at path {} """.format(markupsafe.escape(path)) if not last_seen and path else '', - """with a file size of {} bytes""".format(size) if size and (last_seen or path) else '', - """last seen with a file size of {} bytes""".format(size) if size and not (last_seen or path) else '', + f"""and found at path {markupsafe.escape(path)} """ if last_seen and path else '', + f"""last found at path {markupsafe.escape(path)} """ if not last_seen and path else '', + f"""with a file size of {size} bytes""" if size and (last_seen or path) else '', + f"""last seen with a file size of {size} bytes""" if size and not (last_seen or path) else '', """.""" if last_seen or hashes or path or size else '', """Hashes of last seen version:
{}
""".format( - ''.join(['{}: {}'.format(k, v) for k, v in hashes.items()]) + ''.join([f'{k}: {v}' for k, v in hashes.items()]) ) if hashes else '', # TODO: Format better for UI msg ] diff --git a/addons/base/views.py b/addons/base/views.py index 2e061f4f665..e3605206b0b 100644 --- a/addons/base/views.py +++ b/addons/base/views.py @@ -2,12 +2,12 @@ import os import uuid import markupsafe -from future.moves.urllib.parse import quote +from urllib.parse import quote from django.utils import timezone from flask import make_response from flask import request -import furl +from furl import furl import jwe import jwt import waffle @@ -66,7 +66,7 @@ # import so that associated listener is instantiated and gets emails from website.notifications.events.files import FileEvent # noqa -ERROR_MESSAGES = {'FILE_GONE': u""" +ERROR_MESSAGES = {'FILE_GONE': """ @@ -77,7 +77,7 @@It was deleted by {deleted_by} on {deleted_on}.
""", - 'FILE_GONE_ACTOR_UNKNOWN': u""" + 'FILE_GONE_ACTOR_UNKNOWN': """ @@ -88,7 +88,7 @@It was deleted on {deleted_on}.
""", - 'DONT_KNOW': u""" + 'DONT_KNOW': """ @@ -96,7 +96,7 @@File not found at {provider}.
""", - 'BLAME_PROVIDER': u""" + 'BLAME_PROVIDER': """ @@ -108,7 +108,7 @@You may wish to verify this through {provider}'s website.
""", - 'FILE_SUSPENDED': u""" + 'FILE_SUSPENDED': """ @@ -167,11 +167,26 @@ def get_addon_user_config(**kwargs): return addon.to_json(user) +def _download_is_from_mfr(waterbutler_data): + metrics_data = waterbutler_data['metrics'] + uri = metrics_data['uri'] + is_render_uri = furl(uri or '').query.params.get('mode') == 'render' + return ( + # This header is sent for download requests that + # originate from MFR, e.g. for the code pygments renderer + request.headers.get('X-Cos-Mfr-Render-Request', None) or + # Need to check the URI in order to account + # for renderers that send XHRs from the + # rendered content, e.g. PDFs + is_render_uri + ) + + def make_auth(user): if user is not None: return { 'id': user._id, - 'email': '{}@osf.io'.format(user._id), + 'email': f'{user._id}@osf.io', 'name': user.fullname, } return {} @@ -221,7 +236,13 @@ def get_auth(auth, **kwargs): resource=resource, provider_name=provider_name, file_version=file_version, ) - _enqueue_metrics(file_version=file_version, file_node=file_node, action=action, auth=auth) + _enqueue_metrics( + file_version=file_version, + file_node=file_node, + action=action, + auth=auth, + from_mfr=_download_is_from_mfr(waterbutler_data), + ) # Construct the response payload including the JWT return _construct_payload( @@ -374,13 +395,13 @@ def _get_osfstorage_file_version_and_node( return file_version, file_node -def _enqueue_metrics(file_version, file_node, action, auth): +def _enqueue_metrics(file_version, file_node, action, auth, from_mfr=False): if not file_version: return if action == 'render': file_signals.file_viewed.send(auth=auth, fileversion=file_version, file_node=file_node) - elif action == 'download': + elif action == 'download' and not from_mfr: file_signals.file_downloaded.send(auth=auth, fileversion=file_version, file_node=file_node) return @@ -420,7 +441,7 @@ def _construct_payload(auth, resource, credentials, waterbutler_settings): # Encrypt the encoded JWT with JWE decoded_encrypted_jwt = jwe.encrypt( - encoded_jwt, + encoded_jwt.encode(), WATERBUTLER_JWE_KEY ).decode() @@ -437,10 +458,10 @@ def _construct_payload(auth, resource, credentials, waterbutler_settings): 'create_folder': NodeLog.FOLDER_CREATED, } -DOWNLOAD_ACTIONS = set([ +DOWNLOAD_ACTIONS = { 'download_file', 'download_zip', -]) +} @must_be_signed @no_auto_transaction @@ -817,19 +838,19 @@ def addon_view_or_download_file(auth, path, provider, **kwargs): object_text = markupsafe.escape(getattr(target, 'project_or_component', 'this object')) raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={ 'message_short': 'Bad Request', - 'message_long': 'The {} add-on containing {} is no longer connected to {}.'.format(provider_safe, path_safe, object_text) + 'message_long': f'The {provider_safe} add-on containing {path_safe} is no longer connected to {object_text}.' }) if not node_addon.has_auth: raise HTTPError(http_status.HTTP_401_UNAUTHORIZED, data={ 'message_short': 'Unauthorized', - 'message_long': 'The {} add-on containing {} is no longer authorized.'.format(provider_safe, path_safe) + 'message_long': f'The {provider_safe} add-on containing {path_safe} is no longer authorized.' }) if not node_addon.complete: raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={ 'message_short': 'Bad Request', - 'message_long': 'The {} add-on containing {} is no longer configured.'.format(provider_safe, path_safe) + 'message_long': f'The {provider_safe} add-on containing {path_safe} is no longer configured.' }) savepoint_id = transaction.savepoint() @@ -915,7 +936,7 @@ def addon_view_or_download_file(auth, path, provider, **kwargs): format = extras.get('format') _, extension = os.path.splitext(file_node.name) # avoid rendering files with the same format type. - if format and '.{}'.format(format.lower()) != extension.lower(): + if format and f'.{format.lower()}' != extension.lower(): return redirect('{}/export?format={}&url={}'.format(get_mfr_url(target, provider), format, quote(file_node.generate_waterbutler_url( **dict(extras, direct=None, version=version.identifier, _internal=extras.get('mode') == 'render') )))) @@ -935,10 +956,11 @@ def addon_view_or_download_file(auth, path, provider, **kwargs): if len(request.path.strip('/').split('/')) > 1: guid = file_node.get_guid(create=True) - return redirect(furl.furl('/{}/'.format(guid._id)).set(args=extras).url) + # NOTE: furl encoding to be verified later + return redirect(furl(f'/{guid._id}/', args=extras).url) if isinstance(target, Preprint): # Redirecting preprint file guids to the preprint detail page - return redirect('/{}/'.format(target._id)) + return redirect(f'/{target._id}/') return addon_view_file(auth, target, file_node, version) @@ -983,7 +1005,7 @@ def addon_view_or_download_quickfile(**kwargs): 'message_short': 'File Not Found', 'message_long': 'The requested file could not be found.' }) - return proxy_url('/project/{}/files/osfstorage/{}/'.format(file_.target._id, fid)) + return proxy_url(f'/project/{file_.target._id}/files/osfstorage/{fid}/') def addon_view_file(auth, node, file_node, version): # TODO: resolve circular import issue @@ -1006,8 +1028,9 @@ def addon_view_file(auth, node, file_node, version): else: sharejs_uuid = None - internal_furl = furl.furl(settings.INTERNAL_DOMAIN) - download_url = furl.furl(request.url).set( + internal_furl = furl(settings.INTERNAL_DOMAIN) + download_url = furl( + request.url, netloc=internal_furl.netloc, args=dict(request.args, **{ 'direct': None, @@ -1018,7 +1041,9 @@ def addon_view_file(auth, node, file_node, version): ) mfr_url = get_mfr_url(node, file_node.provider) - render_url = furl.furl(mfr_url).set( + # NOTE: furl encoding to be verified later + render_url = furl( + mfr_url, path=['render'], args={'url': download_url.url} ) diff --git a/addons/bitbucket/__init__.py b/addons/bitbucket/__init__.py index 5563ac92f53..e69de29bb2d 100644 --- a/addons/bitbucket/__init__.py +++ b/addons/bitbucket/__init__.py @@ -1 +0,0 @@ -default_app_config = 'addons.bitbucket.apps.BitbucketAddonConfig' diff --git a/addons/bitbucket/api.py b/addons/bitbucket/api.py index e7176b6395b..4f1176d9a9d 100644 --- a/addons/bitbucket/api.py +++ b/addons/bitbucket/api.py @@ -1,4 +1,4 @@ -from future.moves.urllib.parse import urlencode +from urllib.parse import urlencode from addons.bitbucket import settings @@ -15,7 +15,7 @@ def __init__(self, access_token=None): @property def _default_headers(self): if self.access_token: - return {'Authorization': 'Bearer {}'.format(self.access_token)} + return {'Authorization': f'Bearer {self.access_token}'} return {} @property diff --git a/addons/bitbucket/apps.py b/addons/bitbucket/apps.py index 12ed98d9222..b03cce6f32e 100644 --- a/addons/bitbucket/apps.py +++ b/addons/bitbucket/apps.py @@ -54,7 +54,7 @@ def bitbucket_hgrid_data(node_settings, auth, **kwargs): 'fetch': node_settings.owner.api_url + 'bitbucket/hgrid/' + (ref or ''), 'branch': node_settings.owner.api_url + 'bitbucket/hgrid/root/', 'zip': node_settings.owner.api_url + 'bitbucket/zipball/' + (ref or ''), - 'repo': 'https://bitbucket.com/{0}/{1}/branch/'.format(node_settings.user, node_settings.repo) + 'repo': f'https://bitbucket.com/{node_settings.user}/{node_settings.repo}/branch/' } branch_names = [each['name'] for each in branches] @@ -80,6 +80,7 @@ def bitbucket_hgrid_data(node_settings, auth, **kwargs): class BitbucketAddonConfig(BaseAddonAppConfig): + default = True name = 'addons.bitbucket' label = 'addons_bitbucket' full_name = 'Bitbucket' diff --git a/addons/bitbucket/migrations/0001_initial.py b/addons/bitbucket/migrations/0001_initial.py index dfde90f1fe5..e15bde518bd 100644 --- a/addons/bitbucket/migrations/0001_initial.py +++ b/addons/bitbucket/migrations/0001_initial.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals import addons.base.models from django.db import migrations, models diff --git a/addons/bitbucket/migrations/0002_auto_20220817_1915.py b/addons/bitbucket/migrations/0002_auto_20220817_1915.py index 6646f72e6c5..d443615930b 100644 --- a/addons/bitbucket/migrations/0002_auto_20220817_1915.py +++ b/addons/bitbucket/migrations/0002_auto_20220817_1915.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals from django.conf import settings from django.db import migrations, models diff --git a/addons/bitbucket/migrations/0003_alter_nodesettings_external_account_and_more.py b/addons/bitbucket/migrations/0003_alter_nodesettings_external_account_and_more.py new file mode 100644 index 00000000000..4015fe9da1b --- /dev/null +++ b/addons/bitbucket/migrations/0003_alter_nodesettings_external_account_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.13 on 2024-07-15 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0021_preprint_custom_publication_citation'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('addons_bitbucket', '0002_auto_20220817_1915'), + ] + + operations = [ + migrations.AlterField( + model_name='nodesettings', + name='external_account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.externalaccount'), + ), + migrations.AlterField( + model_name='nodesettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.abstractnode'), + ), + migrations.AlterField( + model_name='usersettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_user_settings', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/addons/bitbucket/models.py b/addons/bitbucket/models.py index b9f22b79ce2..50ebc39a2de 100644 --- a/addons/bitbucket/models.py +++ b/addons/bitbucket/models.py @@ -1,13 +1,12 @@ -# -*- coding: utf-8 -*- - import markupsafe from django.db import models from addons.base import exceptions -from addons.base.models import (BaseOAuthNodeSettings, BaseOAuthUserSettings, - BaseStorageAddon) - +from addons.base.models import ( + BaseOAuthNodeSettings, + BaseOAuthUserSettings, + BaseStorageAddon,) from addons.bitbucket.api import BitbucketClient from addons.bitbucket.serializer import BitbucketSerializer from addons.bitbucket import settings as bitbucket_settings @@ -34,7 +33,7 @@ class BitbucketFile(BitbucketFileNode, File): def touch(self, auth_header, revision=None, commitSha=None, branch=None, **kwargs): revision = revision or commitSha or branch - return super(BitbucketFile, self).touch(auth_header, revision=revision, **kwargs) + return super().touch(auth_header, revision=revision, **kwargs) @property def _hashes(self): @@ -133,7 +132,7 @@ def folder_id(self): @property def folder_name(self): if self.complete: - return '{}/{}'.format(self.user, self.repo) + return f'{self.user}/{self.repo}' return None @property @@ -178,7 +177,7 @@ def deauthorize(self, auth=None, log=True): self.clear_auth() def delete(self, save=False): - super(NodeSettings, self).delete(save=False) + super().delete(save=False) self.deauthorize(log=False) if save: self.save() @@ -186,9 +185,7 @@ def delete(self, save=False): @property def repo_url(self): if self.user and self.repo: - return 'https://bitbucket.org/{0}/{1}/'.format( - self.user, self.repo - ) + return f'https://bitbucket.org/{self.user}/{self.repo}/' @property def short_url(self): @@ -211,7 +208,7 @@ def fetch_access_token(self): # TODO: Delete me and replace with serialize_settings / Knockout def to_json(self, user): - ret = super(NodeSettings, self).to_json(user) + ret = super().to_json(user) user_settings = user.get_addon('bitbucket') ret.update({ 'user_has_auth': user_settings and user_settings.has_auth, @@ -238,7 +235,7 @@ def to_json(self, user): 'node_has_auth': True, 'bitbucket_user': self.user or '', 'bitbucket_repo': self.repo or '', - 'bitbucket_repo_full_name': '{0} / {1}'.format(self.user, self.repo) if (self.user and self.repo) else '', + 'bitbucket_repo_full_name': f'{self.user} / {self.repo}' if (self.user and self.repo) else '', 'auth_osf_name': owner.fullname, 'auth_osf_url': owner.url, 'auth_osf_id': owner._id, @@ -274,14 +271,14 @@ def create_waterbutler_log(self, auth, action, metadata): try: sha = metadata['extra']['commitSha'] urls = { - 'view': '{0}?commitSha={1}'.format(url, sha), - 'download': '{0}?action=download&commitSha={1}'.format(url, sha) + 'view': f'{url}?commitSha={sha}', + 'download': f'{url}?action=download&commitSha={sha}' } except KeyError: pass self.owner.add_log( - 'bitbucket_{0}'.format(action), + f'bitbucket_{action}', auth=auth, params={ 'project': self.owner.parent_id, @@ -371,7 +368,7 @@ def before_remove_contributor_message(self, node, removed): """ try: - message = (super(NodeSettings, self).before_remove_contributor_message(node, removed) + + message = (super().before_remove_contributor_message(node, removed) + 'You can download the contents of this repository before removing ' 'this contributor here.'.format( url=node.api_url + 'bitbucket/tarball/' @@ -397,8 +394,8 @@ def after_remove_contributor(self, node, removed, auth=None): self.user_settings = None self.save() message = ( - u'Because the Bitbucket add-on for {category} "{title}" was authenticated ' - u'by {user}, authentication information has been deleted.' + 'Because the Bitbucket add-on for {category} "{title}" was authenticated ' + 'by {user}, authentication information has been deleted.' ).format( category=markupsafe.escape(node.category_display), title=markupsafe.escape(node.title), @@ -408,7 +405,7 @@ def after_remove_contributor(self, node, removed, auth=None): if not auth or auth.user != removed: url = node.web_url_for('node_setting') message += ( - u' You can re-authenticate on the Settings page.' + ' You can re-authenticate on the Settings page.' ).format(url=url) # return message @@ -424,7 +421,7 @@ def after_fork(self, node, fork, user, save=True): :param bool save: Save settings after callback :return tuple: Tuple of cloned settings and alert message """ - clone = super(NodeSettings, self).after_fork( + clone = super().after_fork( node, fork, user, save=False ) diff --git a/addons/bitbucket/requirements.txt b/addons/bitbucket/requirements.txt deleted file mode 100644 index e1eb62f260f..00000000000 --- a/addons/bitbucket/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -# no requirements diff --git a/addons/bitbucket/routes.py b/addons/bitbucket/routes.py index 07a28748446..9deb07f2428 100644 --- a/addons/bitbucket/routes.py +++ b/addons/bitbucket/routes.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - from framework.routing import Rule, json_renderer from addons.bitbucket import views diff --git a/addons/bitbucket/serializer.py b/addons/bitbucket/serializer.py index 6797ffd5749..65cc76d84b7 100644 --- a/addons/bitbucket/serializer.py +++ b/addons/bitbucket/serializer.py @@ -18,7 +18,7 @@ def credentials_are_valid(self, user_settings, client): def serialized_folder(self, node_settings): return { 'path': node_settings.repo, - 'name': '{0} / {1}'.format(node_settings.user, node_settings.repo), + 'name': f'{node_settings.user} / {node_settings.repo}', } @property diff --git a/addons/bitbucket/settings/__init__.py b/addons/bitbucket/settings/__init__.py index 2b2f98881f6..40f955c5a78 100644 --- a/addons/bitbucket/settings/__init__.py +++ b/addons/bitbucket/settings/__init__.py @@ -7,4 +7,4 @@ try: from .local import * # noqa except ImportError: - logger.warn('No local.py settings file found') + logger.warning('No local.py settings file found') diff --git a/addons/bitbucket/tests/factories.py b/addons/bitbucket/tests/factories.py index d5416c7c938..7220c54cbd7 100644 --- a/addons/bitbucket/tests/factories.py +++ b/addons/bitbucket/tests/factories.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - from factory import Sequence, SubFactory from factory.django import DjangoModelFactory from osf_tests.factories import ExternalAccountFactory, ProjectFactory, UserFactory @@ -9,8 +7,8 @@ class BitbucketAccountFactory(ExternalAccountFactory): provider = 'bitbucket' - provider_id = Sequence(lambda n: 'id-{0}'.format(n)) - oauth_key = Sequence(lambda n: 'key-{0}'.format(n)) + provider_id = Sequence(lambda n: f'id-{n}') + oauth_key = Sequence(lambda n: f'key-{n}') display_name = 'abc' diff --git a/addons/bitbucket/tests/test_models.py b/addons/bitbucket/tests/test_models.py index b2c38d0cd9f..bfa759e23e5 100644 --- a/addons/bitbucket/tests/test_models.py +++ b/addons/bitbucket/tests/test_models.py @@ -1,9 +1,6 @@ -# -*- coding: utf-8 -*- - -import mock +from unittest import mock import pytest import unittest -from nose.tools import * # noqa from tests.base import OsfTestCase, get_default_metaschema from osf_tests.factories import ( @@ -58,21 +55,21 @@ def test_serialize_settings(self): # common storage addons. settings = self.node_settings.serialize_waterbutler_settings() expected = {'owner': self.node_settings.user, 'repo': self.node_settings.repo} - assert_equal(settings, expected) + assert settings == expected @mock.patch( 'addons.bitbucket.models.UserSettings.revoke_remote_oauth_access', mock.PropertyMock() ) def test_complete_has_auth_not_verified(self): - super(TestNodeSettings, self).test_complete_has_auth_not_verified() + super().test_complete_has_auth_not_verified() @mock.patch('addons.bitbucket.api.BitbucketClient.repos') @mock.patch('addons.bitbucket.api.BitbucketClient.team_repos') def test_to_json(self, mock_repos, mock_team_repos): mock_repos.return_value = [] mock_team_repos.return_value = [] - super(TestNodeSettings, self).test_to_json() + super().test_to_json() @mock.patch('addons.bitbucket.api.BitbucketClient.repos') @mock.patch('addons.bitbucket.api.BitbucketClient.team_repos') @@ -80,11 +77,11 @@ def test_to_json_user_is_owner(self, mock_repos, mock_team_repos): mock_repos.return_value = [] mock_team_repos.return_value = [] result = self.node_settings.to_json(self.user) - assert_true(result['user_has_auth']) - assert_equal(result['bitbucket_user'], 'abc') - assert_true(result['is_owner']) - assert_true(result['valid_credentials']) - assert_equal(result.get('repo_names', None), []) + assert result['user_has_auth'] + assert result['bitbucket_user'] == 'abc' + assert result['is_owner'] + assert result['valid_credentials'] + assert result.get('repo_names', None) == [] @mock.patch('addons.bitbucket.api.BitbucketClient.repos') @mock.patch('addons.bitbucket.api.BitbucketClient.team_repos') @@ -93,11 +90,11 @@ def test_to_json_user_is_not_owner(self, mock_repos, mock_team_repos): mock_team_repos.return_value = [] not_owner = UserFactory() result = self.node_settings.to_json(not_owner) - assert_false(result['user_has_auth']) - assert_equal(result['bitbucket_user'], 'abc') - assert_false(result['is_owner']) - assert_true(result['valid_credentials']) - assert_equal(result.get('repo_names', None), None) + assert not result['user_has_auth'] + assert result['bitbucket_user'] == 'abc' + assert not result['is_owner'] + assert result['valid_credentials'] + assert result.get('repo_names', None) == None class TestUserSettings(models.OAuthAddonUserSettingTestSuiteMixin, unittest.TestCase): @@ -107,14 +104,14 @@ class TestUserSettings(models.OAuthAddonUserSettingTestSuiteMixin, unittest.Test ExternalAccountFactory = BitbucketAccountFactory def test_public_id(self): - assert_equal(self.user.external_accounts.first().display_name, self.user_settings.public_id) + assert self.user.external_accounts.first().display_name == self.user_settings.public_id class TestCallbacks(OsfTestCase): def setUp(self): - super(TestCallbacks, self).setUp() + super().setUp() self.project = ProjectFactory() self.consolidated_auth = Auth(self.project.creator) @@ -144,7 +141,7 @@ def test_before_make_public(self, mock_repo): mock_repo.side_effect = NotFoundError result = self.node_settings.before_make_public(self.project) - assert_is(result, None) + assert result is None @mock.patch('addons.bitbucket.api.BitbucketClient.repo') def test_before_page_load_osf_public_bb_public(self, mock_repo): @@ -156,7 +153,7 @@ def test_before_page_load_osf_public_bb_public(self, mock_repo): user=self.node_settings.user, repo=self.node_settings.repo, ) - assert_false(message) + assert not message @mock.patch('addons.bitbucket.api.BitbucketClient.repo') def test_before_page_load_osf_public_bb_private(self, mock_repo): @@ -168,8 +165,8 @@ def test_before_page_load_osf_public_bb_private(self, mock_repo): user=self.node_settings.user, repo=self.node_settings.repo, ) - assert_true(message) - assert_in('Users can view the contents of this private Bitbucket repository through this public project.', message[0]) + assert message + assert 'Users can view the contents of this private Bitbucket repository through this public project.' in message[0] @mock.patch('addons.bitbucket.api.BitbucketClient.repo') def test_before_page_load_repo_deleted(self, mock_repo): @@ -181,8 +178,8 @@ def test_before_page_load_repo_deleted(self, mock_repo): user=self.node_settings.user, repo=self.node_settings.repo, ) - assert_true(message) - assert_in('has been deleted.', message[0]) + assert message + assert 'has been deleted.' in message[0] @mock.patch('addons.bitbucket.api.BitbucketClient.repo') def test_before_page_load_osf_private_bb_public(self, mock_repo): @@ -192,8 +189,8 @@ def test_before_page_load_osf_private_bb_public(self, mock_repo): user=self.node_settings.user, repo=self.node_settings.repo, ) - assert_true(message) - assert_in('The files in this Bitbucket repo can be viewed on Bitbucket', message[0]) + assert message + assert 'The files in this Bitbucket repo can be viewed on Bitbucket' in message[0] @mock.patch('addons.bitbucket.api.BitbucketClient.repo') def test_before_page_load_osf_private_bb_private(self, mock_repo): @@ -203,85 +200,70 @@ def test_before_page_load_osf_private_bb_private(self, mock_repo): user=self.node_settings.user, repo=self.node_settings.repo, ) - assert_false(message) + assert not message def test_before_page_load_not_contributor(self): message = self.node_settings.before_page_load(self.project, UserFactory()) - assert_false(message) + assert not message def test_before_page_load_not_logged_in(self): message = self.node_settings.before_page_load(self.project, None) - assert_false(message) + assert not message def test_before_remove_contributor_authenticator(self): message = self.node_settings.before_remove_contributor( self.project, self.project.creator ) - assert_true(message) + assert message def test_before_remove_contributor_not_authenticator(self): message = self.node_settings.before_remove_contributor( self.project, self.non_authenticator ) - assert_false(message) + assert not message def test_after_remove_contributor_authenticator_self(self): message = self.node_settings.after_remove_contributor( self.project, self.project.creator, self.consolidated_auth ) - assert_equal( - self.node_settings.user_settings, - None - ) - assert_true(message) - assert_not_in('You can re-authenticate', message) + assert self.node_settings.user_settings is None + assert message + assert 'You can re-authenticate' not in message def test_after_remove_contributor_authenticator_not_self(self): auth = Auth(user=self.non_authenticator) message = self.node_settings.after_remove_contributor( self.project, self.project.creator, auth ) - assert_equal( - self.node_settings.user_settings, - None - ) - assert_true(message) - assert_in('You can re-authenticate', message) + assert self.node_settings.user_settings is None + assert message + assert 'You can re-authenticate' in message def test_after_remove_contributor_not_authenticator(self): self.node_settings.after_remove_contributor( self.project, self.non_authenticator, self.consolidated_auth ) - assert_not_equal( - self.node_settings.user_settings, - None, - ) + assert self.node_settings.user_settings is not None def test_after_fork_authenticator(self): fork = ProjectFactory() clone = self.node_settings.after_fork( self.project, fork, self.project.creator, ) - assert_equal( - self.node_settings.user_settings, - clone.user_settings, - ) + assert self.node_settings.user_settings == clone.user_settings def test_after_fork_not_authenticator(self): fork = ProjectFactory() clone = self.node_settings.after_fork( self.project, fork, self.non_authenticator, ) - assert_equal( - clone.user_settings, - None, - ) + assert clone.user_settings is None def test_after_delete(self): self.project.remove_node(Auth(user=self.project.creator)) # Ensure that changes to node settings have been saved self.node_settings.reload() - assert_true(self.node_settings.user_settings is None) + assert self.node_settings.user_settings is None @mock.patch('website.archiver.tasks.archive') def test_does_not_get_copied_to_registrations(self, mock_archive): @@ -290,4 +272,4 @@ def test_does_not_get_copied_to_registrations(self, mock_archive): auth=Auth(user=self.project.creator), draft_registration=DraftRegistrationFactory(branched_from=self.project), ) - assert_false(registration.has_addon('bitbucket')) + assert not registration.has_addon('bitbucket') diff --git a/addons/bitbucket/tests/test_serializer.py b/addons/bitbucket/tests/test_serializer.py index 2f804ec6980..008625ab909 100644 --- a/addons/bitbucket/tests/test_serializer.py +++ b/addons/bitbucket/tests/test_serializer.py @@ -1,8 +1,6 @@ -# -*- coding: utf-8 -*- """Serializer tests for the Bitbucket addon.""" -import mock -from nose.tools import * # noqa (PEP8 asserts) +from unittest import mock import pytest from tests.base import OsfTestCase diff --git a/addons/bitbucket/tests/test_views.py b/addons/bitbucket/tests/test_views.py index 13725b3f2c0..51cac6012b5 100644 --- a/addons/bitbucket/tests/test_views.py +++ b/addons/bitbucket/tests/test_views.py @@ -1,12 +1,10 @@ -# -*- coding: utf-8 -*- from rest_framework import status as http_status -import mock +from unittest import mock import datetime import unittest import pytest -from nose.tools import * # noqa (PEP8 asserts) from tests.base import OsfTestCase, get_default_metaschema from osf_tests.factories import ( ProjectFactory, @@ -38,7 +36,7 @@ class TestBitbucketAuthViews(BitbucketAddonTestCase, OAuthAddonAuthViewsTestCase mock.PropertyMock() ) def test_delete_external_account(self): - super(TestBitbucketAuthViews, self).test_delete_external_account() + super().test_delete_external_account() class TestBitbucketConfigViews(BitbucketAddonTestCase, OAuthAddonConfigViewsTestCaseMixin, OsfTestCase): @@ -49,14 +47,14 @@ class TestBitbucketConfigViews(BitbucketAddonTestCase, OAuthAddonConfigViewsTest ## Overrides ## def setUp(self): - super(TestBitbucketConfigViews, self).setUp() + super().setUp() self.mock_access_token = mock.patch('addons.bitbucket.models.BitbucketProvider.fetch_access_token') self.mock_access_token.return_value = mock.Mock() self.mock_access_token.start() def tearDown(self): self.mock_access_token.stop() - super(TestBitbucketConfigViews, self).tearDown() + super().tearDown() def test_folder_list(self): # BB only lists root folder (repos), this test is superfluous @@ -68,23 +66,20 @@ def test_set_config(self, mock_account, mock_repo): # BB selects repos, not folders, so this needs to be overriden mock_account.return_value = mock.Mock() mock_repo.return_value = 'repo_name' - url = self.project.api_url_for('{0}_set_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.post_json(url, { + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config') + res = self.app.post(url, json={ 'bitbucket_user': 'octocat', 'bitbucket_repo': 'repo_name', }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.project.reload() - assert_equal( - self.project.logs.latest().action, - '{0}_repo_linked'.format(self.ADDON_SHORT_NAME) - ) + assert self.project.logs.latest().action == f'{self.ADDON_SHORT_NAME}_repo_linked' class TestBitbucketViews(OsfTestCase): def setUp(self): - super(TestBitbucketViews, self).setUp() + super().setUp() self.user = AuthUserFactory() self.consolidated_auth = Auth(user=self.user) @@ -140,17 +135,14 @@ def test_get_refs_defaults(self, mock_account, mock_default_branch, mock_repo, m mock_branches.return_value = bitbucket_mock.branches.return_value branch, sha, branches = utils.get_refs(self.node_settings) - assert_equal( - branch, - bitbucket_mock.repo_default_branch.return_value - ) - assert_equal(sha, self._get_sha_for_branch(branch=None)) # Get refs for default branch + assert branch == bitbucket_mock.repo_default_branch.return_value + assert sha == self._get_sha_for_branch(branch=None) # Get refs for default branch expected_branches = [ {'name': x['name'], 'sha': x['target']['hash']} for x in bitbucket_mock.branches.return_value ] - assert_equal(branches, expected_branches) + assert branches == expected_branches @mock.patch('addons.bitbucket.api.BitbucketClient.branches') @mock.patch('addons.bitbucket.api.BitbucketClient.repo') @@ -164,47 +156,47 @@ def test_get_refs_branch(self, mock_account, mock_default_branch, mock_repo, moc mock_branches.return_value = bitbucket_mock.branches.return_value branch, sha, branches = utils.get_refs(self.node_settings, 'master') - assert_equal(branch, 'master') + assert branch == 'master' branch_sha = self._get_sha_for_branch('master') - assert_equal(sha, branch_sha) + assert sha == branch_sha expected_branches = [ {'name': x['name'], 'sha': x['target']['hash']} for x in bitbucket_mock.branches.return_value ] - assert_equal(branches, expected_branches) + assert branches == expected_branches def test_before_fork(self): url = self.project.api_url + 'fork/before/' - res = self.app.get(url, auth=self.user.auth).maybe_follow() - assert_equal(len(res.json['prompts']), 1) + res = self.app.get(url, auth=self.user.auth, follow_redirects=True) + assert len(res.json['prompts']) == 1 def test_before_register(self): url = self.project.api_url + 'beforeregister/' - res = self.app.get(url, auth=self.user.auth).maybe_follow() - assert_true('Bitbucket' in res.json['prompts'][1]) + res = self.app.get(url, auth=self.user.auth, follow_redirects=True) + assert 'Bitbucket' in res.json['prompts'][1] @mock.patch('addons.bitbucket.models.NodeSettings.external_account') def test_get_refs_sha_no_branch(self, mock_account): - with assert_raises(HTTPError): + with pytest.raises(HTTPError): utils.get_refs(self.node_settings, sha='12345') def check_hook_urls(self, urls, node, path, sha): url = node.web_url_for('addon_view_or_download_file', path=path, provider='bitbucket') expected_urls = { - 'view': '{0}?ref={1}'.format(url, sha), - 'download': '{0}?action=download&ref={1}'.format(url, sha) + 'view': f'{url}?ref={sha}', + 'download': f'{url}?action=download&ref={sha}' } - assert_equal(urls['view'], expected_urls['view']) - assert_equal(urls['download'], expected_urls['download']) + assert urls['view'] == expected_urls['view'] + assert urls['download'] == expected_urls['download'] class TestBitbucketSettings(OsfTestCase): def setUp(self): - super(TestBitbucketSettings, self).setUp() + super().setUp() self.bitbucket = create_mock_bitbucket(user='fred', private=False) self.project = ProjectFactory() self.project.save() @@ -228,21 +220,21 @@ def test_link_repo(self, mock_account, mock_repo): mock_repo.return_value = bitbucket_mock.repo.return_value url = self.project.api_url + 'bitbucket/settings/' - self.app.post_json( + self.app.post( url, - { + json={ 'bitbucket_user': 'queen', 'bitbucket_repo': 'night at the opera', }, - auth=self.auth - ).maybe_follow() + auth=self.auth, follow_redirects=True + ) self.project.reload() self.node_settings.reload() - assert_equal(self.node_settings.user, 'queen') - assert_equal(self.node_settings.repo, 'night at the opera') - assert_equal(self.project.logs.latest().action, 'bitbucket_repo_linked') + assert self.node_settings.user == 'queen' + assert self.node_settings.repo == 'night at the opera' + assert self.project.logs.latest().action == 'bitbucket_repo_linked' @mock.patch('addons.bitbucket.api.BitbucketClient.repo') @mock.patch('addons.bitbucket.models.NodeSettings.external_account') @@ -254,19 +246,20 @@ def test_link_repo_no_change(self, mock_account, mock_repo): log_count = self.project.logs.count() url = self.project.api_url + 'bitbucket/settings/' - self.app.post_json( + self.app.post( url, - { + json={ 'bitbucket_user': 'Queen', 'bitbucket_repo': 'Sheer-Heart-Attack', }, - auth=self.auth - ).maybe_follow() + auth=self.auth, + follow_redirects=True + ) self.project.reload() self.node_settings.reload() - assert_equal(self.project.logs.count(), log_count) + assert self.project.logs.count() == log_count @mock.patch('addons.bitbucket.api.BitbucketClient.repo') @mock.patch('addons.bitbucket.models.NodeSettings.external_account') @@ -275,17 +268,17 @@ def test_link_repo_non_existent(self, mock_account, mock_repo): mock_repo.return_value = None url = self.project.api_url + 'bitbucket/settings/' - res = self.app.post_json( + res = self.app.post( url, - { + json={ 'bitbucket_user': 'queen', 'bitbucket_repo': 'night at the opera', }, auth=self.auth, - expect_errors=True - ).maybe_follow() + follow_redirects=True + ) - assert_equal(res.status_code, 400) + assert res.status_code == 400 @mock.patch('addons.bitbucket.api.BitbucketClient.branches') def test_link_repo_registration(self, mock_branches): @@ -299,31 +292,31 @@ def test_link_repo_registration(self, mock_branches): ) url = registration.api_url + 'bitbucket/settings/' - res = self.app.post_json( + res = self.app.post( url, - { + json={ 'bitbucket_user': 'queen', 'bitbucket_repo': 'night at the opera', }, auth=self.auth, - expect_errors=True - ).maybe_follow() + follow_redirects=True + ) - assert_equal(res.status_code, 400) + assert res.status_code == 400 def test_deauthorize(self): url = self.project.api_url + 'bitbucket/user_auth/' - self.app.delete(url, auth=self.auth).maybe_follow() + self.app.delete(url, auth=self.auth, follow_redirects=True) self.project.reload() self.node_settings.reload() - assert_equal(self.node_settings.user, None) - assert_equal(self.node_settings.repo, None) - assert_equal(self.node_settings.user_settings, None) + assert self.node_settings.user == None + assert self.node_settings.repo == None + assert self.node_settings.user_settings == None - assert_equal(self.project.logs.latest().action, 'bitbucket_node_deauthorized') + assert self.project.logs.latest().action == 'bitbucket_node_deauthorized' if __name__ == '__main__': diff --git a/addons/bitbucket/tests/utils.py b/addons/bitbucket/tests/utils.py index 6f1312fae01..bfedc8d0ac6 100644 --- a/addons/bitbucket/tests/utils.py +++ b/addons/bitbucket/tests/utils.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock from addons.bitbucket.api import BitbucketClient @@ -13,7 +13,7 @@ class BitbucketAddonTestCase(OAuthAddonTestCaseMixin, AddonTestCase): Provider = BitbucketProvider def set_node_settings(self, settings): - super(BitbucketAddonTestCase, self).set_node_settings(settings) + super().set_node_settings(settings) settings.repo = 'abc' settings.user = 'octo-cat' @@ -36,9 +36,9 @@ def create_mock_bitbucket(user='octo-cat', private=False): 'owner': {'username': user}, } bitbucket_mock.repos.return_value = [ - {'full_name': '{}/cow-problems-app'.format(user)}, - {'full_name': '{}/duck-problems-app'.format(user)}, - {'full_name': '{}/horse-problems-app'.format(user)}, + {'full_name': f'{user}/cow-problems-app'}, + {'full_name': f'{user}/duck-problems-app'}, + {'full_name': f'{user}/horse-problems-app'}, ] bitbucket_mock.team_repos.return_value = [ {'full_name': 'team-barn-devs/pig-problems-app'}, diff --git a/addons/bitbucket/utils.py b/addons/bitbucket/utils.py index a7f8b213941..d289e056669 100644 --- a/addons/bitbucket/utils.py +++ b/addons/bitbucket/utils.py @@ -1,4 +1,4 @@ -from future.moves.urllib.parse import unquote_plus +from urllib.parse import unquote_plus from rest_framework import status as http_status from framework.exceptions import HTTPError diff --git a/addons/bitbucket/views.py b/addons/bitbucket/views.py index df555703c34..0409595c9e1 100644 --- a/addons/bitbucket/views.py +++ b/addons/bitbucket/views.py @@ -1,5 +1,4 @@ """Views for the node settings page.""" -# -*- coding: utf-8 -*- from rest_framework import status as http_status import logging diff --git a/addons/boa/__init__.py b/addons/boa/__init__.py index b593ad68bd2..e69de29bb2d 100644 --- a/addons/boa/__init__.py +++ b/addons/boa/__init__.py @@ -1 +0,0 @@ -default_app_config = 'addons.boa.apps.BoaAddonAppConfig' diff --git a/addons/boa/apps.py b/addons/boa/apps.py index bd6ac6eea13..bc0e4fa78a5 100644 --- a/addons/boa/apps.py +++ b/addons/boa/apps.py @@ -8,6 +8,7 @@ class BoaAddonAppConfig(BaseAddonAppConfig): + default = True name = 'addons.boa' label = 'addons_boa' full_name = 'Boa' diff --git a/addons/boa/migrations/0002_alter_nodesettings_external_account_and_more.py b/addons/boa/migrations/0002_alter_nodesettings_external_account_and_more.py new file mode 100644 index 00000000000..e3a52c86d27 --- /dev/null +++ b/addons/boa/migrations/0002_alter_nodesettings_external_account_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.13 on 2024-07-15 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0021_preprint_custom_publication_citation'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('addons_boa', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='nodesettings', + name='external_account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.externalaccount'), + ), + migrations.AlterField( + model_name='nodesettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.abstractnode'), + ), + migrations.AlterField( + model_name='usersettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_user_settings', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/addons/boa/models.py b/addons/boa/models.py index bde51f59b2a..01578a89bdc 100644 --- a/addons/boa/models.py +++ b/addons/boa/models.py @@ -17,7 +17,7 @@ class BoaProvider(BasicAuthProviderMixin): def __init__(self, account=None, host=None, username=None, password=None): if username: username = username.lower() - super(BoaProvider, self).__init__(account=account, host=host, username=username, password=password) + super().__init__(account=account, host=host, username=username, password=password) def __repr__(self): return '<{name}: {status}>'.format( @@ -32,7 +32,7 @@ class UserSettings(BaseOAuthUserSettings): serializer = BoaSerializer def to_json(self, user): - ret = super(UserSettings, self).to_json(user) + ret = super().to_json(user) ret['hosts'] = DEFAULT_HOSTS return ret diff --git a/addons/boa/requirements.txt b/addons/boa/requirements.txt deleted file mode 100644 index 196f2a7d505..00000000000 --- a/addons/boa/requirements.txt +++ /dev/null @@ -1,5 +0,0 @@ -# Requirements for the boa add-on -boa-api==0.1.14 - -# Requirements for running asyncio in celery, using 3.4.1 for Python 3.6 compatibility -asgiref==3.4.1 diff --git a/addons/boa/serializer.py b/addons/boa/serializer.py index 7a3275ff170..42c738e62a8 100644 --- a/addons/boa/serializer.py +++ b/addons/boa/serializer.py @@ -52,19 +52,19 @@ def addon_serialized_urls(self): @property def serialized_node_settings(self): - result = super(BoaSerializer, self).serialized_node_settings + result = super().serialized_node_settings result['hosts'] = DEFAULT_HOSTS return result @property def serialized_user_settings(self): - result = super(BoaSerializer, self).serialized_user_settings + result = super().serialized_user_settings result['hosts'] = DEFAULT_HOSTS return result def serialize_settings(self, node_settings, current_user, client=None): if client is not None: sentry.log_message('Client ignored for Boa Serializer in serialize_settings()') - ret = super(BoaSerializer, self).serialize_settings(node_settings, current_user, client=client) + ret = super().serialize_settings(node_settings, current_user, client=client) ret['hosts'] = DEFAULT_HOSTS return ret diff --git a/addons/boa/settings/__init__.py b/addons/boa/settings/__init__.py index 90f189ec18d..6deafee2808 100644 --- a/addons/boa/settings/__init__.py +++ b/addons/boa/settings/__init__.py @@ -7,4 +7,4 @@ try: from addons.boa.settings.local import * # noqa except ImportError: - logger.warn('No local.py settings file found') + logger.warning('No local.py settings file found') diff --git a/addons/boa/tests/factories.py b/addons/boa/tests/factories.py index c432eea1c24..6be5ccc2f42 100644 --- a/addons/boa/tests/factories.py +++ b/addons/boa/tests/factories.py @@ -1,4 +1,5 @@ -from factory import DjangoModelFactory, Sequence, SubFactory +from factory import Sequence, SubFactory +from factory.django import DjangoModelFactory from addons.boa.models import UserSettings, NodeSettings from osf_tests.factories import UserFactory, ProjectFactory, ExternalAccountFactory @@ -12,9 +13,9 @@ class BoaAccountFactory(ExternalAccountFactory): provider = 'boa' provider_name = 'Fake Boa Provider' - provider_id = Sequence(lambda n: '{0}:{1}-{2}'.format(BOA_HOST, BOA_USERNAME, n)) - profile_url = Sequence(lambda n: 'http://localhost:9999/{0}/boa'.format(n)) - oauth_secret = Sequence(lambda n: 'secret-{0}'.format(n)) + provider_id = Sequence(lambda n: f'{BOA_HOST}:{BOA_USERNAME}-{n}') + profile_url = Sequence(lambda n: f'http://localhost:9999/{n}/boa') + oauth_secret = Sequence(lambda n: f'secret-{n}') oauth_key = BOA_PASSWORD display_name = 'Fake Boa' diff --git a/addons/boa/tests/test_serializer.py b/addons/boa/tests/test_serializer.py index 100c044797f..058ed018533 100644 --- a/addons/boa/tests/test_serializer.py +++ b/addons/boa/tests/test_serializer.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock import pytest from tests.base import OsfTestCase @@ -18,11 +18,11 @@ def setUp(self): self.mock_credentials = mock.patch('addons.boa.serializer.BoaSerializer.credentials_are_valid') self.mock_credentials.return_value = True self.mock_credentials.start() - super(TestBoaSerializer, self).setUp() + super().setUp() def tearDown(self): self.mock_credentials.stop() - super(TestBoaSerializer, self).tearDown() + super().tearDown() def test_serialize_settings_authorized_folder_is_set(self): self.set_provider_id(pid='foo') diff --git a/addons/boa/tests/test_tasks.py b/addons/boa/tests/test_tasks.py index e0807823a9f..a4842d6c417 100644 --- a/addons/boa/tests/test_tasks.py +++ b/addons/boa/tests/test_tasks.py @@ -1,8 +1,7 @@ -from asynctest import TestCase as AsyncTestCase from boaapi.boa_client import BoaException from boaapi.status import CompilerStatus, ExecutionStatus from http.client import HTTPMessage -import mock +from unittest import mock import pytest from unittest.mock import ANY, MagicMock from urllib.error import HTTPError @@ -21,13 +20,13 @@ class AsyncMock(MagicMock): async def __call__(self, *args, **kwargs): - return super(AsyncMock, self).__call__(*args, **kwargs) + return super().__call__(*args, **kwargs) class TestBoaErrorHandling(OsfTestCase): def setUp(self): - super(TestBoaErrorHandling, self).setUp() + super().setUp() self.error_message = 'fake-error-message' self.user_username = 'fake-user-username' self.user_fullname = 'fake-user-fullname' @@ -40,7 +39,7 @@ def setUp(self): self.job_id = '1a2b3c4d5e6f7g8' def tearDown(self): - super(TestBoaErrorHandling, self).tearDown() + super().tearDown() def test_boa_error_code(self): assert BoaErrorCode.NO_ERROR == -1 @@ -95,7 +94,7 @@ def test_handle_boa_error(self): class TestSubmitToBoa(OsfTestCase): def setUp(self): - super(TestSubmitToBoa, self).setUp() + super().setUp() self.host = 'http://locahost:9999/boa/?q=boa/api' self.username = 'fake-boa-username' self.password = 'fake-boa-password' @@ -109,7 +108,7 @@ def setUp(self): self.output_upload_url = f'http://localhost:7777/v1/resources/{self.project_guid}/providers/osfstorage/?kind=file' def tearDown(self): - super(TestSubmitToBoa, self).tearDown() + super().tearDown() def test_submit_to_boa_async_called(self): with mock.patch( @@ -135,10 +134,11 @@ def test_submit_to_boa_async_called(self): @pytest.mark.django_db -class TestSubmitToBoaAsync(OsfTestCase, AsyncTestCase): +@pytest.mark.asyncio +class TestSubmitToBoaAsync(OsfTestCase): def setUp(self): - super(TestSubmitToBoaAsync, self).setUp() + super().setUp() self.host = 'http://locahost:9999/boa/?q=boa/api' self.username = 'fake-boa-username' self.password = 'fake-boa-password' @@ -168,7 +168,7 @@ def setUp(self): boa_settings.MAX_JOB_WAITING_TIME = DEFAULT_MAX_JOB_WAITING_TIME def tearDown(self): - super(TestSubmitToBoaAsync, self).tearDown() + super().tearDown() async def test_submit_success(self): with mock.patch('osf.models.user.OSFUser.objects.get', return_value=self.user), \ diff --git a/addons/boa/tests/test_views.py b/addons/boa/tests/test_views.py index 1cd4516cf86..3989386f739 100644 --- a/addons/boa/tests/test_views.py +++ b/addons/boa/tests/test_views.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock import pytest from rest_framework import status as http_status @@ -29,7 +29,7 @@ def test_oauth_finish(self): class TestConfigViews(BoaBasicAuthAddonTestCase, OAuthAddonConfigViewsTestCaseMixin, OsfTestCase): def setUp(self): - super(TestConfigViews, self).setUp() + super().setUp() self.mock_boa_client_login = mock.patch('boaapi.boa_client.BoaClient.login') self.mock_boa_client_close = mock.patch('boaapi.boa_client.BoaClient.close') self.mock_boa_client_login.start() @@ -38,7 +38,7 @@ def setUp(self): def tearDown(self): self.mock_boa_client_close.stop() self.mock_boa_client_login.stop() - super(TestConfigViews, self).tearDown() + super().tearDown() def test_folder_list(self): """Not applicable to remote computing add-ons.""" @@ -77,7 +77,7 @@ def test_get_config_owner_with_external_account(self): assert self.node_settings.external_account is not None assert serialized['validCredentials'] is True - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=self.user.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -93,7 +93,7 @@ def test_get_config_owner_without_external_account(self): assert self.node_settings.external_account is None assert serialized['validCredentials'] is False - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=self.user.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -112,7 +112,7 @@ def test_get_config_write_contrib_with_external_account(self): assert self.node_settings.external_account is not None assert serialized['validCredentials'] is True - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=user_write.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -130,7 +130,7 @@ def test_get_config_write_contrib_without_external_account(self): assert self.node_settings.external_account is None assert serialized['validCredentials'] is False - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=user_write.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -149,7 +149,7 @@ def test_get_config_admin_contrib_with_external_account(self): assert self.node_settings.external_account is not None assert serialized['validCredentials'] is True - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=user_admin.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -167,7 +167,7 @@ def test_get_config_admin_contrib_without_external_account(self): assert self.node_settings.external_account is None assert serialized['validCredentials'] is False - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=user_admin.auth) assert res.status_code == http_status.HTTP_200_OK assert 'result' in res.json @@ -178,9 +178,9 @@ def test_get_config_read_contrib_with_valid_credentials(self): user_read_only = AuthUserFactory() self.project.add_contributor(user_read_only, permissions=permissions.READ, auth=self.auth, save=True) - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') with mock.patch.object(type(self.Serializer()), 'credentials_are_valid', return_value=True): - res = self.app.get(url, auth=user_read_only.auth, expect_errors=True) + res = self.app.get(url, auth=user_read_only.auth) assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_get_config_read_contrib_without_valid_credentials(self): @@ -188,16 +188,16 @@ def test_get_config_read_contrib_without_valid_credentials(self): user_read_only = AuthUserFactory() self.project.add_contributor(user_read_only, permissions=permissions.READ, auth=self.auth, save=True) - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') with mock.patch.object(type(self.Serializer()), 'credentials_are_valid', return_value=False): - res = self.app.get(url, auth=user_read_only.auth, expect_errors=True) + res = self.app.get(url, auth=user_read_only.auth) assert res.status_code == http_status.HTTP_403_FORBIDDEN class TestBoaSubmitViews(BoaBasicAuthAddonTestCase, OsfTestCase): def setUp(self): - super(TestBoaSubmitViews, self).setUp() + super().setUp() self.folder_name = 'fake_boa_folder' self.file_name = 'fake_boa_file.boa' self.file_size = 255 @@ -238,7 +238,7 @@ def setUp(self): } def tearDown(self): - super(TestBoaSubmitViews, self).tearDown() + super().tearDown() def test_boa_submit_job_from_addon_root(self): with mock.patch('addons.boa.tasks.submit_to_boa.s', return_value=BoaErrorCode.NO_ERROR) as mock_submit_s: @@ -247,7 +247,7 @@ def test_boa_submit_job_from_addon_root(self): addon_root_url = waterbutler_api_url_for(self.project._id, 'osfstorage', _internal=True, base_url=base_url) upload_url_root = f'{addon_root_url}?kind=file' url = self.project.api_url_for('boa_submit_job') - res = self.app.post_json(url, self.payload_addon_root, auth=self.user.auth) + res = self.app.post(url, json=self.payload_addon_root, auth=self.user.auth) assert res.status_code == http_status.HTTP_200_OK mock_submit_s.assert_called_with( BOA_HOST, @@ -267,7 +267,7 @@ def test_boa_submit_job_from_sub_folder(self): with mock.patch('addons.boa.tasks.submit_to_boa.s', return_value=BoaErrorCode.NO_ERROR) as mock_submit_s: self.node_settings.set_auth(self.external_account, self.user) url = self.project.api_url_for('boa_submit_job') - res = self.app.post_json(url, self.payload_sub_folder, auth=self.user.auth) + res = self.app.post(url, json=self.payload_sub_folder, auth=self.user.auth) assert res.status_code == http_status.HTTP_200_OK mock_submit_s.assert_called_with( BOA_HOST, @@ -289,7 +289,7 @@ def test_boa_submit_job_admin_contrib(self): user_admin = AuthUserFactory() self.project.add_contributor(user_admin, permissions=permissions.ADMIN, auth=self.auth, save=True) url = self.project.api_url_for('boa_submit_job') - res = self.app.post_json(url, self.payload_sub_folder, auth=user_admin.auth) + res = self.app.post(url, json=self.payload_sub_folder, auth=user_admin.auth) assert res.status_code == http_status.HTTP_200_OK mock_submit_s.assert_called_with( BOA_HOST, @@ -311,7 +311,7 @@ def test_boa_submit_job_write_contrib(self): user_write = AuthUserFactory() self.project.add_contributor(user_write, permissions=permissions.WRITE, auth=self.auth, save=True) url = self.project.api_url_for('boa_submit_job') - res = self.app.post_json(url, self.payload_sub_folder, auth=user_write.auth) + res = self.app.post(url, json=self.payload_sub_folder, auth=user_write.auth) assert res.status_code == http_status.HTTP_200_OK mock_submit_s.assert_called_with( BOA_HOST, @@ -333,6 +333,6 @@ def test_boa_submit_job_read_contrib(self): user_read_only = AuthUserFactory() self.project.add_contributor(user_read_only, permissions=permissions.READ, auth=self.auth, save=True) url = self.project.api_url_for('boa_submit_job') - res = self.app.post_json(url, self.payload_sub_folder, auth=user_read_only.auth, expect_errors=True) + res = self.app.post(url, json=self.payload_sub_folder, auth=user_read_only.auth) assert res.status_code == http_status.HTTP_403_FORBIDDEN mock_submit_s.assert_not_called() diff --git a/addons/boa/tests/utils.py b/addons/boa/tests/utils.py index b03e9d803c9..c5ad1f474b0 100644 --- a/addons/boa/tests/utils.py +++ b/addons/boa/tests/utils.py @@ -3,7 +3,7 @@ from addons.boa.tests.factories import BoaAccountFactory, BoaNodeSettingsFactory, BoaUserSettingsFactory -class BoaAddonTestCaseBaseMixin(object): +class BoaAddonTestCaseBaseMixin: short_name = 'boa' full_name = 'Boa' @@ -22,12 +22,12 @@ class BoaAddonTestCaseBaseMixin(object): class BoaBasicAuthAddonTestCase(BoaAddonTestCaseBaseMixin, OAuthAddonTestCaseMixin, AddonTestCase): def __init__(self, *args, **kwargs): - super(BoaBasicAuthAddonTestCase, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.auth = None self.external_account = None def set_user_settings(self, settings): - super(BoaBasicAuthAddonTestCase, self).set_user_settings(settings) + super().set_user_settings(settings) def set_node_settings(self, settings): - super(BoaBasicAuthAddonTestCase, self).set_node_settings(settings) + super().set_node_settings(settings) diff --git a/addons/boa/views.py b/addons/boa/views.py index 2b82d4c91bd..05cf37729c5 100644 --- a/addons/boa/views.py +++ b/addons/boa/views.py @@ -50,7 +50,7 @@ def boa_add_user_account(auth, **kwargs): except ValidationError: provider.account = ExternalAccount.objects.get( provider=provider.short_name, - provider_id='{}:{}'.format(BOA_API_ENDPOINT, username).lower() + provider_id=f'{BOA_API_ENDPOINT}:{username}'.lower() ) if provider.account.oauth_key != password: provider.account.oauth_key = password diff --git a/addons/box/__init__.py b/addons/box/__init__.py index 305007bec08..e69de29bb2d 100644 --- a/addons/box/__init__.py +++ b/addons/box/__init__.py @@ -1 +0,0 @@ -default_app_config = 'addons.box.apps.BoxAddonAppConfig' diff --git a/addons/box/apps.py b/addons/box/apps.py index 0637591fa66..d7563b94fca 100644 --- a/addons/box/apps.py +++ b/addons/box/apps.py @@ -6,6 +6,7 @@ class BoxAddonAppConfig(BaseAddonAppConfig): + default = True name = 'addons.box' label = 'addons_box' full_name = 'Box' diff --git a/addons/box/migrations/0001_initial.py b/addons/box/migrations/0001_initial.py index f2053798e95..0b9a6c40caf 100644 --- a/addons/box/migrations/0001_initial.py +++ b/addons/box/migrations/0001_initial.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals import addons.base.models from django.db import migrations, models diff --git a/addons/box/migrations/0002_auto_20220817_1915.py b/addons/box/migrations/0002_auto_20220817_1915.py index 58be50b8bea..3805ee039bc 100644 --- a/addons/box/migrations/0002_auto_20220817_1915.py +++ b/addons/box/migrations/0002_auto_20220817_1915.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals from django.conf import settings from django.db import migrations, models diff --git a/addons/box/migrations/0003_alter_nodesettings_external_account_and_more.py b/addons/box/migrations/0003_alter_nodesettings_external_account_and_more.py new file mode 100644 index 00000000000..0db0015ea20 --- /dev/null +++ b/addons/box/migrations/0003_alter_nodesettings_external_account_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.13 on 2024-07-15 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0021_preprint_custom_publication_citation'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('addons_box', '0002_auto_20220817_1915'), + ] + + operations = [ + migrations.AlterField( + model_name='nodesettings', + name='external_account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.externalaccount'), + ), + migrations.AlterField( + model_name='nodesettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.abstractnode'), + ), + migrations.AlterField( + model_name='usersettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_user_settings', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/addons/box/models.py b/addons/box/models.py index 88108a78bc1..78002b122df 100644 --- a/addons/box/models.py +++ b/addons/box/models.py @@ -71,7 +71,7 @@ def handle_callback(self, response): return { 'provider_id': about['id'], 'display_name': about['name'], - 'profile_url': 'https://app.box.com/profile/{0}'.format(about['id']) + 'profile_url': f'https://app.box.com/profile/{about["id"]}' } @@ -118,7 +118,7 @@ def api(self): @property def display_name(self): - return '{0}: {1}'.format(self.config.full_name, self.folder_id) + return f'{self.config.full_name}: {self.folder_id}' def fetch_full_folder_path(self): return self.folder_path @@ -134,7 +134,7 @@ def get_folders(self, **kwargs): 'name': '/ (Full Box)', 'urls': { # 'folders': node.api_url_for('box_folder_list', folderId=0), - 'folders': api_v2_url('nodes/{}/addons/box/folders/'.format(self.owner._id), + 'folders': api_v2_url(f'nodes/{self.owner._id}/addons/box/folders/', params={'id': '0'} ) } @@ -169,7 +169,7 @@ def get_folders(self, **kwargs): 'name': item['name'], 'path': os.path.join(folder_path, item['name']).replace('All Files', ''), 'urls': { - 'folders': api_v2_url('nodes/{}/addons/box/folders/'.format(self.owner._id), + 'folders': api_v2_url(f'nodes/{self.owner._id}/addons/box/folders/', params={'id': item['id']} ) } @@ -237,7 +237,7 @@ def serialize_waterbutler_settings(self): def create_waterbutler_log(self, auth, action, metadata): self.owner.add_log( - 'box_{0}'.format(action), + f'box_{action}', auth=auth, params={ 'path': metadata['materialized'], diff --git a/addons/box/requirements.txt b/addons/box/requirements.txt deleted file mode 100644 index c1b869fddf2..00000000000 --- a/addons/box/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -boxsdk==1.5.5 diff --git a/addons/box/routes.py b/addons/box/routes.py index a73302bd638..b29d650902d 100644 --- a/addons/box/routes.py +++ b/addons/box/routes.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """Box addon routes.""" from framework.routing import Rule, json_renderer diff --git a/addons/box/settings/__init__.py b/addons/box/settings/__init__.py index bce98689d3b..eb5d40c3725 100644 --- a/addons/box/settings/__init__.py +++ b/addons/box/settings/__init__.py @@ -6,4 +6,4 @@ try: from .local import * # noqa except ImportError: - logger.warn('No local.py settings file found') + logger.warning('No local.py settings file found') diff --git a/addons/box/settings/local-dist.py b/addons/box/settings/local-dist.py index 492ed19ff45..38d245c2df6 100644 --- a/addons/box/settings/local-dist.py +++ b/addons/box/settings/local-dist.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """Example Box local settings file. Copy this file to local.py and change these settings. """ diff --git a/addons/box/tests/factories.py b/addons/box/tests/factories.py index bada66e3a54..17b1898e49d 100644 --- a/addons/box/tests/factories.py +++ b/addons/box/tests/factories.py @@ -14,8 +14,8 @@ class BoxAccountFactory(ExternalAccountFactory): provider = 'box' - provider_id = Sequence(lambda n: 'id-{0}'.format(n)) - oauth_key = Sequence(lambda n: 'key-{0}'.format(n)) + provider_id = Sequence(lambda n: f'id-{n}') + oauth_key = Sequence(lambda n: f'key-{n}') expires_at = timezone.now() + relativedelta(seconds=3600) diff --git a/addons/box/tests/test_client.py b/addons/box/tests/test_client.py index 97db6293bd4..e6051137ed5 100644 --- a/addons/box/tests/test_client.py +++ b/addons/box/tests/test_client.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- -from nose.tools import assert_true import pytest import unittest @@ -13,7 +11,7 @@ class TestCore(unittest.TestCase): def setUp(self): - super(TestCore, self).setUp() + super().setUp() self.user = UserFactory() self.user.add_addon('box') @@ -24,4 +22,4 @@ def setUp(self): def test_get_addon_returns_box_user_settings(self): result = self.user.get_addon('box') - assert_true(isinstance(result, UserSettings)) + assert isinstance(result, UserSettings) diff --git a/addons/box/tests/test_models.py b/addons/box/tests/test_models.py index c3a9a78c903..6829b754d23 100644 --- a/addons/box/tests/test_models.py +++ b/addons/box/tests/test_models.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock import unittest import pytest @@ -28,11 +28,11 @@ def setUp(self): return_value=('12235', '/Foo') ) self.mock_data.start() - super(TestBoxNodeSettings, self).setUp() + super().setUp() def tearDown(self): self.mock_data.stop() - super(TestBoxNodeSettings, self).tearDown() + super().tearDown() def test_folder_defaults_to_none(self): node_settings = NodeSettings(user_settings=self.user_settings, owner=factories.ProjectFactory()) @@ -42,11 +42,11 @@ def test_folder_defaults_to_none(self): @mock.patch('addons.box.models.Provider.refresh_oauth_key') def test_serialize_credentials(self, mock_refresh): mock_refresh.return_value = True - super(TestBoxNodeSettings, self).test_serialize_credentials() + super().test_serialize_credentials() @mock.patch('addons.box.models.UserSettings.revoke_remote_oauth_access', mock.PropertyMock()) def test_complete_has_auth_not_verified(self): - super(TestBoxNodeSettings, self).test_complete_has_auth_not_verified() + super().test_complete_has_auth_not_verified() class TestBoxUserSettings(OAuthAddonUserSettingTestSuiteMixin, unittest.TestCase): diff --git a/addons/box/tests/test_serializer.py b/addons/box/tests/test_serializer.py index 1ee4537d4ba..e32adec4faf 100644 --- a/addons/box/tests/test_serializer.py +++ b/addons/box/tests/test_serializer.py @@ -1,6 +1,5 @@ -# -*- coding: utf-8 -*- """Serializer tests for the Box addon.""" -import mock +from unittest import mock import pytest from addons.base.tests.serializers import StorageAddonSerializerTestSuiteMixin @@ -27,11 +26,11 @@ def setUp(self): return_value=True ) self.mock_valid.start() - super(TestBoxSerializer, self).setUp() + super().setUp() def tearDown(self): self.mock_valid.stop() - super(TestBoxSerializer, self).tearDown() + super().tearDown() def set_provider_id(self, pid): self.node_settings.folder_id = pid diff --git a/addons/box/tests/test_views.py b/addons/box/tests/test_views.py index af7d646ae77..f8da949c5bf 100644 --- a/addons/box/tests/test_views.py +++ b/addons/box/tests/test_views.py @@ -1,9 +1,7 @@ -# -*- coding: utf-8 -*- """Views tests for the Box addon.""" from django.utils import timezone from rest_framework import status as http_status -from nose.tools import * # noqa (PEP8 asserts) -import mock +from unittest import mock import pytest from urllib3.exceptions import MaxRetryError @@ -32,18 +30,18 @@ def setUp(self): self.mock_refresh = mock.patch('addons.box.models.Provider.refresh_oauth_key') self.mock_refresh.return_value = True self.mock_refresh.start() - super(TestAuthViews, self).setUp() + super().setUp() def tearDown(self): self.mock_refresh.stop() - super(TestAuthViews, self).tearDown() + super().tearDown() @mock.patch( 'addons.box.models.UserSettings.revoke_remote_oauth_access', mock.PropertyMock() ) def test_delete_external_account(self): - super(TestAuthViews, self).test_delete_external_account() + super().test_delete_external_account() class TestConfigViews(BoxAddonTestCase, views_testing.OAuthAddonConfigViewsTestCaseMixin, OsfTestCase): @@ -62,20 +60,20 @@ def setUp(self): return_value=(self.folder['id'], self.folder['path']) ) self.mock_data.start() - super(TestConfigViews, self).setUp() + super().setUp() def tearDown(self): self.mock_data.stop() - super(TestConfigViews, self).tearDown() + super().tearDown() @mock.patch.object(BoxSerializer, 'credentials_are_valid', return_value=True) def test_import_auth(self, *args): - super(TestConfigViews, self).test_import_auth() + super().test_import_auth() class TestFilebrowserViews(BoxAddonTestCase, OsfTestCase): def setUp(self): - super(TestFilebrowserViews, self).setUp() + super().setUp() self.user.add_addon('box') self.node_settings.external_account = self.user_settings.external_accounts[0] self.node_settings.save() @@ -93,10 +91,10 @@ def test_box_list_folders(self): res = self.app.get(url, auth=self.user.auth) contents = mock_client.folder('', list=True)['item_collection']['entries'] expected = [each for each in contents if each['type'] == 'folder'] - assert_equal(len(res.json), len(expected)) + assert len(res.json) == len(expected) first = res.json[0] - assert_in('kind', first) - assert_equal(first['name'], contents[0]['name']) + assert 'kind' in first + assert first['name'] == contents[0]['name'] @mock.patch('addons.box.models.NodeSettings.folder_id') def test_box_list_folders_if_folder_is_none(self, mock_folder): @@ -104,7 +102,7 @@ def test_box_list_folders_if_folder_is_none(self, mock_folder): mock_folder.__get__ = mock.Mock(return_value=None) url = self.project.api_url_for('box_folder_list') res = self.app.get(url, auth=self.user.auth) - assert_equal(len(res.json), 1) + assert len(res.json) == 1 def test_box_list_folders_if_folder_is_none_and_folders_only(self): with patch_client('addons.box.models.Client'): @@ -115,7 +113,7 @@ def test_box_list_folders_if_folder_is_none_and_folders_only(self): res = self.app.get(url, auth=self.user.auth) contents = mock_client.folder('', list=True)['item_collection']['entries'] expected = [each for each in contents if each['type'] == 'folder'] - assert_equal(len(res.json), len(expected)) + assert len(res.json) == len(expected) def test_box_list_folders_folders_only(self): with patch_client('addons.box.models.Client'): @@ -123,7 +121,7 @@ def test_box_list_folders_folders_only(self): res = self.app.get(url, auth=self.user.auth) contents = mock_client.folder('', list=True)['item_collection']['entries'] expected = [each for each in contents if each['type'] == 'folder'] - assert_equal(len(res.json), len(expected)) + assert len(res.json) == len(expected) def test_box_list_folders_doesnt_include_root(self): with mock.patch('addons.box.models.Client.folder') as folder_mock: @@ -133,22 +131,22 @@ def test_box_list_folders_doesnt_include_root(self): contents = mock_client.folder('', list=True)['item_collection']['entries'] expected = [each for each in contents if each['type'] == 'folder'] - assert_equal(len(res.json), len(expected)) + assert len(res.json) == len(expected) @mock.patch('addons.box.models.Client.folder') def test_box_list_folders_returns_error_if_invalid_path(self, mock_metadata): mock_metadata.side_effect = BoxAPIException(status=404, message='File not found') url = self.project.api_url_for('box_folder_list', folder_id='lolwut') - res = self.app.get(url, auth=self.user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_404_NOT_FOUND) + res = self.app.get(url, auth=self.user.auth) + assert res.status_code == http_status.HTTP_404_NOT_FOUND @mock.patch('addons.box.models.Client.folder') def test_box_list_folders_handles_max_retry_error(self, mock_metadata): mock_response = mock.Mock() url = self.project.api_url_for('box_folder_list', folder_id='fo') mock_metadata.side_effect = MaxRetryError(mock_response, url) - res = self.app.get(url, auth=self.user.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_400_BAD_REQUEST) + res = self.app.get(url, auth=self.user.auth) + assert res.status_code == http_status.HTTP_400_BAD_REQUEST class TestRestrictions(BoxAddonTestCase, OsfTestCase): @@ -179,14 +177,14 @@ def test_restricted_hgrid_data_contents(self, mock_auth): # tries to access a parent folder url = self.project.api_url_for('box_folder_list', path='foo bar') - res = self.app.get(url, auth=self.contrib.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + res = self.app.get(url, auth=self.contrib.auth) + assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_restricted_config_contrib_no_addon(self): url = api_url_for('box_set_config', pid=self.project._primary_key) - res = self.app.put_json(url, {'selected': {'path': 'foo'}}, - auth=self.contrib.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_400_BAD_REQUEST) + res = self.app.put(url, json={'selected': {'path': 'foo'}}, + auth=self.contrib.auth) + assert res.status_code == http_status.HTTP_400_BAD_REQUEST def test_restricted_config_contrib_not_owner(self): # Contributor has box auth, but is not the node authorizer @@ -194,6 +192,6 @@ def test_restricted_config_contrib_not_owner(self): self.contrib.save() url = api_url_for('box_set_config', pid=self.project._primary_key) - res = self.app.put_json(url, {'selected': {'path': 'foo'}}, - auth=self.contrib.auth, expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + res = self.app.put(url, json={'selected': {'path': 'foo'}}, + auth=self.contrib.auth) + assert res.status_code == http_status.HTTP_403_FORBIDDEN diff --git a/addons/box/tests/utils.py b/addons/box/tests/utils.py index 793d16c5b1f..7b4c942258e 100644 --- a/addons/box/tests/utils.py +++ b/addons/box/tests/utils.py @@ -1,5 +1,4 @@ -# -*- coding: utf-8 -*- -import mock +from unittest import mock from contextlib import contextmanager from addons.base.tests.base import OAuthAddonTestCaseMixin, AddonTestCase @@ -14,7 +13,7 @@ class BoxAddonTestCase(OAuthAddonTestCaseMixin, AddonTestCase): Provider = Provider def set_node_settings(self, settings): - super(BoxAddonTestCase, self).set_node_settings(settings) + super().set_node_settings(settings) settings.folder_id = '1234567890' settings.folder_name = 'Foo' @@ -88,64 +87,64 @@ def set_node_settings(self, settings): 'revision': 220191 }, { - u'bytes': 0, - u'icon': u'folder', - u'is_dir': True, - u'modified': u'Sat, 22 Mar 2014 05:40:29 +0000', - u'path': u'/datasets/New Folder', - u'rev': u'3fed51f002c12fc', - u'revision': 67032351, - u'root': u'box', - u'size': u'0 bytes', - u'thumb_exists': False + 'bytes': 0, + 'icon': 'folder', + 'is_dir': True, + 'modified': 'Sat, 22 Mar 2014 05:40:29 +0000', + 'path': '/datasets/New Folder', + 'rev': '3fed51f002c12fc', + 'revision': 67032351, + 'root': 'box', + 'size': '0 bytes', + 'thumb_exists': False } ], 'revision': 29007 }, 'metadata_single': { - u'id': 'id', - u'bytes': 74, - u'client_mtime': u'Mon, 13 Jan 2014 20:24:15 +0000', - u'icon': u'page_white', - u'is_dir': False, - u'mime_type': u'text/csv', - u'modified': u'Fri, 21 Mar 2014 05:46:36 +0000', - u'path': '/datasets/foo.txt', - u'rev': u'a2149fb64', - u'revision': 10, - u'root': u'app_folder', - u'size': u'74 bytes', - u'thumb_exists': False + 'id': 'id', + 'bytes': 74, + 'client_mtime': 'Mon, 13 Jan 2014 20:24:15 +0000', + 'icon': 'page_white', + 'is_dir': False, + 'mime_type': 'text/csv', + 'modified': 'Fri, 21 Mar 2014 05:46:36 +0000', + 'path': '/datasets/foo.txt', + 'rev': 'a2149fb64', + 'revision': 10, + 'root': 'app_folder', + 'size': '74 bytes', + 'thumb_exists': False }, - 'revisions': [{u'bytes': 0, - u'client_mtime': u'Wed, 31 Dec 1969 23:59:59 +0000', - u'icon': u'page_white_picture', - u'is_deleted': True, - u'is_dir': False, - u'mime_type': u'image/png', - u'modified': u'Tue, 25 Mar 2014 03:39:13 +0000', - u'path': u'/svs-v-barks.png', - u'rev': u'3fed741002c12fc', - u'revision': 67032897, - u'root': u'box', - u'size': u'0 bytes', - u'thumb_exists': True}, - {u'bytes': 151164, - u'client_mtime': u'Sat, 13 Apr 2013 21:56:36 +0000', - u'icon': u'page_white_picture', - u'is_dir': False, - u'mime_type': u'image/png', - u'modified': u'Tue, 25 Mar 2014 01:45:51 +0000', - u'path': u'/svs-v-barks.png', - u'rev': u'3fed61a002c12fc', - u'revision': 67032602, - u'root': u'box', - u'size': u'147.6 KB', - u'thumb_exists': True}] + 'revisions': [{'bytes': 0, + 'client_mtime': 'Wed, 31 Dec 1969 23:59:59 +0000', + 'icon': 'page_white_picture', + 'is_deleted': True, + 'is_dir': False, + 'mime_type': 'image/png', + 'modified': 'Tue, 25 Mar 2014 03:39:13 +0000', + 'path': '/svs-v-barks.png', + 'rev': '3fed741002c12fc', + 'revision': 67032897, + 'root': 'box', + 'size': '0 bytes', + 'thumb_exists': True}, + {'bytes': 151164, + 'client_mtime': 'Sat, 13 Apr 2013 21:56:36 +0000', + 'icon': 'page_white_picture', + 'is_dir': False, + 'mime_type': 'image/png', + 'modified': 'Tue, 25 Mar 2014 01:45:51 +0000', + 'path': '/svs-v-barks.png', + 'rev': '3fed61a002c12fc', + 'revision': 67032602, + 'root': 'box', + 'size': '147.6 KB', + 'thumb_exists': True}] } -class MockBox(object): +class MockBox: def put_file(self, full_path, file_obj, overwrite=False, parent_rev=None): return mock_responses['put_file'] diff --git a/addons/box/views.py b/addons/box/views.py index e808b079536..7e3694f75ae 100644 --- a/addons/box/views.py +++ b/addons/box/views.py @@ -1,5 +1,4 @@ """Views for the node settings page.""" -# -*- coding: utf-8 -*- from flask import request from addons.base import generic_views diff --git a/addons/dataverse/__init__.py b/addons/dataverse/__init__.py index 3041d2b08d3..e69de29bb2d 100644 --- a/addons/dataverse/__init__.py +++ b/addons/dataverse/__init__.py @@ -1 +0,0 @@ -default_app_config = 'addons.dataverse.apps.DataverseAddonAppConfig' diff --git a/addons/dataverse/apps.py b/addons/dataverse/apps.py index 1221a979139..15b04f40f15 100644 --- a/addons/dataverse/apps.py +++ b/addons/dataverse/apps.py @@ -11,6 +11,7 @@ class DataverseAddonAppConfig(BaseAddonAppConfig): + default = True name = 'addons.dataverse' label = 'addons_dataverse' full_name = 'Dataverse' diff --git a/addons/dataverse/migrations/0001_initial.py b/addons/dataverse/migrations/0001_initial.py index 8f0bdbe9345..d3a1ec3a73c 100644 --- a/addons/dataverse/migrations/0001_initial.py +++ b/addons/dataverse/migrations/0001_initial.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals import addons.base.models from django.db import migrations, models diff --git a/addons/dataverse/migrations/0002_auto_20220817_1915.py b/addons/dataverse/migrations/0002_auto_20220817_1915.py index bb10ee76ec4..1ba459bd7a6 100644 --- a/addons/dataverse/migrations/0002_auto_20220817_1915.py +++ b/addons/dataverse/migrations/0002_auto_20220817_1915.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals from django.conf import settings from django.db import migrations, models diff --git a/addons/dataverse/migrations/0003_alter_nodesettings_external_account_and_more.py b/addons/dataverse/migrations/0003_alter_nodesettings_external_account_and_more.py new file mode 100644 index 00000000000..d792caf1d49 --- /dev/null +++ b/addons/dataverse/migrations/0003_alter_nodesettings_external_account_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.13 on 2024-07-15 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0021_preprint_custom_publication_citation'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('addons_dataverse', '0002_auto_20220817_1915'), + ] + + operations = [ + migrations.AlterField( + model_name='nodesettings', + name='external_account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.externalaccount'), + ), + migrations.AlterField( + model_name='nodesettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.abstractnode'), + ), + migrations.AlterField( + model_name='usersettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_user_settings', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/addons/dataverse/models.py b/addons/dataverse/models.py index f586a99a958..1b4753d7cfa 100644 --- a/addons/dataverse/models.py +++ b/addons/dataverse/models.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from rest_framework import status as http_status from addons.base import exceptions as addon_errors @@ -71,14 +70,14 @@ def update(self, revision, data, save=True, user=None): return version -class DataverseProvider(object): +class DataverseProvider: """An alternative to `ExternalProvider` not tied to OAuth""" name = 'Dataverse' short_name = 'dataverse' serializer = DataverseSerializer def __init__(self, account=None): - super(DataverseProvider, self).__init__() # this does exactly nothing... + super().__init__() # this does exactly nothing... # provide an unauthenticated session by default self.account = account @@ -165,7 +164,7 @@ def set_folder(self, dataverse, dataset, auth=None): def _get_fileobj_child_metadata(self, filenode, user, cookie=None, version=None): try: - return super(NodeSettings, self)._get_fileobj_child_metadata(filenode, user, cookie=cookie, version=version) + return super()._get_fileobj_child_metadata(filenode, user, cookie=cookie, version=version) except HTTPError as e: # The Dataverse API returns a 404 if the dataset has no published files if e.code == http_status.HTTP_404_NOT_FOUND and version == 'latest-published': @@ -215,7 +214,7 @@ def serialize_waterbutler_settings(self): def create_waterbutler_log(self, auth, action, metadata): url = self.owner.web_url_for('addon_view_or_download_file', path=metadata['path'], provider='dataverse') self.owner.add_log( - 'dataverse_{0}'.format(action), + f'dataverse_{action}', auth=auth, params={ 'project': self.owner.parent_id, diff --git a/addons/dataverse/requirements.txt b/addons/dataverse/requirements.txt deleted file mode 100644 index 5bf59c7dae2..00000000000 --- a/addons/dataverse/requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ -# Allow for optional timeout parameter. -# https://github.com/IQSS/dataverse-client-python/pull/27 -#-e git+https://github.com/IQSS/dataverse-client-python.git@b6aec5bc3fd1cc3199e3c97ac9c71705b54c3d11#egg=dataverse --e git+https://github.com/CenterForOpenScience/dataverse-client-python.git@5a9a8cabc9a521bd86efc0105075c5a4816a2ca5#egg=dataverse diff --git a/addons/dataverse/serializer.py b/addons/dataverse/serializer.py index 0535580827f..0b8d66abceb 100644 --- a/addons/dataverse/serializer.py +++ b/addons/dataverse/serializer.py @@ -12,11 +12,11 @@ class DataverseSerializer(OAuthAddonSerializer): # Include host information with more informative labels / formatting def serialize_account(self, external_account): - ret = super(DataverseSerializer, self).serialize_account(external_account) + ret = super().serialize_account(external_account) host = external_account.oauth_key ret.update({ 'host': host, - 'host_url': 'https://{0}'.format(host), + 'host_url': f'https://{host}', }) return ret @@ -38,7 +38,7 @@ def serialized_urls(self): addon_urls = self.addon_serialized_urls # Make sure developer returns set of needed urls for url in self.REQUIRED_URLS: - assert url in addon_urls, "addon_serilized_urls must include key '{0}'".format(url) + assert url in addon_urls, f"addon_serilized_urls must include key '{url}'" ret.update(addon_urls) return ret @@ -55,13 +55,13 @@ def addon_serialized_urls(self): 'deauthorize': node.api_url_for('dataverse_deauthorize_node'), 'getDatasets': node.api_url_for('dataverse_get_datasets'), 'datasetPrefix': 'https://doi.org/', - 'dataversePrefix': 'http://{0}/dataverse/'.format(host), + 'dataversePrefix': f'http://{host}/dataverse/', 'accounts': api_url_for('dataverse_account_list'), } @property def serialized_node_settings(self): - result = super(DataverseSerializer, self).serialized_node_settings + result = super().serialized_node_settings result['hosts'] = DEFAULT_HOSTS # Update with Dataverse specific fields diff --git a/addons/dataverse/settings/__init__.py b/addons/dataverse/settings/__init__.py index bce98689d3b..eb5d40c3725 100644 --- a/addons/dataverse/settings/__init__.py +++ b/addons/dataverse/settings/__init__.py @@ -6,4 +6,4 @@ try: from .local import * # noqa except ImportError: - logger.warn('No local.py settings file found') + logger.warning('No local.py settings file found') diff --git a/addons/dataverse/tests/factories.py b/addons/dataverse/tests/factories.py index 2d13177d5fe..6a67eb8fd5c 100644 --- a/addons/dataverse/tests/factories.py +++ b/addons/dataverse/tests/factories.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """Factory boy factories for the Dataverse addon.""" import factory from factory.django import DjangoModelFactory @@ -10,8 +9,8 @@ class DataverseAccountFactory(ExternalAccountFactory): provider = 'dataverse' provider_name = 'Dataverse' - provider_id = factory.Sequence(lambda n: 'id-{0}'.format(n)) - oauth_key = factory.Sequence(lambda n: 'key-{0}'.format(n)) + provider_id = factory.Sequence(lambda n: f'id-{n}') + oauth_key = factory.Sequence(lambda n: f'key-{n}') display_name = 'foo.bar.baz' oauth_secret = 'doremi-abc-123' diff --git a/addons/dataverse/tests/test_client.py b/addons/dataverse/tests/test_client.py index 37835869435..19812d1c8b3 100644 --- a/addons/dataverse/tests/test_client.py +++ b/addons/dataverse/tests/test_client.py @@ -1,8 +1,4 @@ -import mock -from nose.tools import ( - assert_equal, assert_raises, assert_true, - assert_false, assert_in, assert_is, assert_is_none -) +from unittest import mock import pytest import unittest @@ -25,7 +21,7 @@ class TestClient(DataverseAddonTestCase, unittest.TestCase): def setUp(self): - super(TestClient, self).setUp() + super().setUp() self.host = 'some.host.url' self.token = 'some-fancy-api-token-which-is-long' @@ -45,12 +41,12 @@ def test_connect(self, mock_connection): c = _connect(self.host, self.token) mock_connection.assert_called_once_with(self.host, self.token) - assert_true(c) + assert c @mock.patch('addons.dataverse.client.Connection') def test_connect_fail(self, mock_connection): mock_connection.side_effect = UnauthorizedError() - with assert_raises(UnauthorizedError): + with pytest.raises(UnauthorizedError): _connect(self.host, self.token) mock_connection.assert_called_once_with(self.host, self.token) @@ -61,16 +57,16 @@ def test_connect_or_error(self, mock_connection): c = connect_or_error(self.host, self.token) mock_connection.assert_called_once_with(self.host, self.token) - assert_true(c) + assert c @mock.patch('addons.dataverse.client.Connection') def test_connect_or_error_returns_401_when_client_raises_unauthorized_error(self, mock_connection): mock_connection.side_effect = UnauthorizedError() - with assert_raises(HTTPError) as cm: + with pytest.raises(HTTPError) as cm: connect_or_error(self.host, self.token) mock_connection.assert_called_once_with(self.host, self.token) - assert_equal(cm.exception.code, 401) + assert cm.value.code == 401 @mock.patch('addons.dataverse.client._connect') def test_connect_from_settings(self, mock_connect): @@ -80,12 +76,12 @@ def test_connect_from_settings(self, mock_connect): ) connection = connect_from_settings(node_settings) - assert_true(connection) + assert connection mock_connect.assert_called_once_with(self.host, self.token) def test_connect_from_settings_none(self): connection = connect_from_settings(None) - assert_is_none(connection) + assert connection is None @mock.patch('addons.dataverse.client._connect') def test_connect_from_settings_or_401(self, mock_connect): @@ -95,12 +91,12 @@ def test_connect_from_settings_or_401(self, mock_connect): ) connection = connect_from_settings_or_401(node_settings) - assert_true(connection) + assert connection mock_connect.assert_called_once_with(self.host, self.token) def test_connect_from_settings_or_401_none(self): connection = connect_from_settings_or_401(None) - assert_is_none(connection) + assert connection is None @mock.patch('addons.dataverse.client.Connection') def test_connect_from_settings_or_401_forbidden(self, mock_connection): @@ -110,11 +106,11 @@ def test_connect_from_settings_or_401_forbidden(self, mock_connection): self.host, self.token, ) - with assert_raises(HTTPError) as e: + with pytest.raises(HTTPError) as e: connect_from_settings_or_401(node_settings) mock_connection.assert_called_once_with(self.host, self.token) - assert_equal(e.exception.code, 401) + assert e.value.code == 401 def test_get_files(self): published = False @@ -132,11 +128,11 @@ def test_publish_dataset(self): def test_publish_dataset_unpublished_dataverse(self): type(self.mock_dataverse).is_published = mock.PropertyMock(return_value=False) - with assert_raises(HTTPError) as e: + with pytest.raises(HTTPError) as e: publish_dataset(self.mock_dataset) - assert_false(self.mock_dataset.publish.called) - assert_equal(e.exception.code, 405) + assert not self.mock_dataset.publish.called + assert e.value.code == 405 def test_get_datasets(self): mock_dataset1 = mock.create_autospec(Dataset) @@ -150,23 +146,23 @@ def test_get_datasets(self): ] datasets = get_datasets(self.mock_dataverse) - assert_is(self.mock_dataverse.get_datasets.assert_called_once_with(timeout=settings.REQUEST_TIMEOUT), None) - assert_in(mock_dataset1, datasets) - assert_in(mock_dataset2, datasets) - assert_in(mock_dataset3, datasets) + assert self.mock_dataverse.get_datasets.assert_called_once_with(timeout=settings.REQUEST_TIMEOUT) is None + assert mock_dataset1 in datasets + assert mock_dataset2 in datasets + assert mock_dataset3 in datasets def test_get_datasets_no_dataverse(self): datasets = get_datasets(None) - assert_equal(datasets, []) + assert datasets == [] def test_get_dataset(self): self.mock_dataset.get_state.return_value = 'DRAFT' self.mock_dataverse.get_dataset_by_doi.return_value = self.mock_dataset s = get_dataset(self.mock_dataverse, 'My hdl') - assert_is(self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT), None) + assert self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT) is None - assert_equal(s, self.mock_dataset) + assert s == self.mock_dataset @mock.patch('dataverse.dataverse.requests') def test_get_dataset_calls_patched_timeout_method(self, mock_requests): @@ -177,30 +173,30 @@ def test_get_dataset_calls_patched_timeout_method(self, mock_requests): dataverse.collection.get.return_value = '123' mock_requests.get.side_effect = Exception('Done Testing') - with assert_raises(Exception) as e: + with pytest.raises(Exception) as e: get_dataset(dataverse, 'My hdl') - assert_is(mock_requests.get.assert_called_once_with('123', auth='me', timeout=settings.REQUEST_TIMEOUT), None) - assert_equal(str(e.exception), 'Done Testing') + assert mock_requests.get.assert_called_once_with('123', auth='me', timeout=settings.REQUEST_TIMEOUT) is None + assert str(e.value) == 'Done Testing' def test_get_deaccessioned_dataset(self): self.mock_dataset.get_state.return_value = 'DEACCESSIONED' self.mock_dataverse.get_dataset_by_doi.return_value = self.mock_dataset - with assert_raises(HTTPError) as e: + with pytest.raises(HTTPError) as e: get_dataset(self.mock_dataverse, 'My hdl') - assert_is(self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT), None) - assert_equal(e.exception.code, 410) + assert self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT) is None + assert e.value.code == 410 def test_get_bad_dataset(self): error = UnicodeDecodeError('utf-8', b'', 1, 2, 'jeepers') self.mock_dataset.get_state.side_effect = error self.mock_dataverse.get_dataset_by_doi.return_value = self.mock_dataset - with assert_raises(HTTPError) as e: + with pytest.raises(HTTPError) as e: get_dataset(self.mock_dataverse, 'My hdl') - assert_is(self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT), None) - assert_equal(e.exception.code, 406) + assert self.mock_dataverse.get_dataset_by_doi.assert_called_once_with('My hdl', timeout=settings.REQUEST_TIMEOUT) is None + assert e.value.code == 406 def test_get_dataverses(self): published_dv = mock.create_autospec(Dataverse) @@ -214,9 +210,9 @@ def test_get_dataverses(self): dvs = get_dataverses(self.mock_connection) self.mock_connection.get_dataverses.assert_called_once_with() - assert_in(published_dv, dvs) - assert_in(unpublished_dv, dvs) - assert_equal(len(dvs), 2) + assert published_dv in dvs + assert unpublished_dv in dvs + assert len(dvs) == 2 def test_get_dataverse(self): type(self.mock_dataverse).is_published = mock.PropertyMock(return_value=True) @@ -225,7 +221,7 @@ def test_get_dataverse(self): d = get_dataverse(self.mock_connection, 'ALIAS') self.mock_connection.get_dataverse.assert_called_once_with('ALIAS') - assert_equal(d, self.mock_dataverse) + assert d == self.mock_dataverse def test_get_unpublished_dataverse(self): type(self.mock_dataverse).is_published = mock.PropertyMock(return_value=False) @@ -234,4 +230,4 @@ def test_get_unpublished_dataverse(self): d = get_dataverse(self.mock_connection, 'ALIAS') self.mock_connection.get_dataverse.assert_called_once_with('ALIAS') - assert_equal(d, self.mock_dataverse) + assert d == self.mock_dataverse diff --git a/addons/dataverse/tests/test_logger.py b/addons/dataverse/tests/test_logger.py index 032ca9c0878..6ff9670e41e 100644 --- a/addons/dataverse/tests/test_logger.py +++ b/addons/dataverse/tests/test_logger.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """NodeLogger tests for the Dataverse addon.""" import pytest @@ -16,10 +15,10 @@ class TestDataverseNodeLogger(StorageAddonNodeLoggerTestSuiteMixin, OsfTestCase) NodeLogger = DataverseNodeLogger def setUp(self): - super(TestDataverseNodeLogger, self).setUp() + super().setUp() node_settings = self.node.get_addon(self.addon_short_name) node_settings.dataset = 'fake dataset' node_settings.save() def tearDown(self): - super(TestDataverseNodeLogger, self).tearDown() + super().tearDown() diff --git a/addons/dataverse/tests/test_model.py b/addons/dataverse/tests/test_model.py index 1c53b57ff5c..6054ea40dac 100644 --- a/addons/dataverse/tests/test_model.py +++ b/addons/dataverse/tests/test_model.py @@ -1,5 +1,4 @@ -from nose.tools import * # noqa -import mock +from unittest import mock import pytest import unittest @@ -45,7 +44,7 @@ def test_does_not_get_copied_to_registrations(self, mock_archive): auth=Auth(user=self.node.creator), draft_registration=DraftRegistrationFactory(branched_from=self.node), ) - assert_false(registration.has_addon('dataverse')) + assert not registration.has_addon('dataverse') ## Overrides ## @@ -59,32 +58,28 @@ def test_create_log(self): metadata={'path': filename, 'materialized': filename}, ) self.node.reload() - assert_equal(self.node.logs.count(), nlog + 1) - assert_equal( - self.node.logs.latest().action, - '{0}_{1}'.format(self.short_name, action), - ) - assert_equal( - self.node.logs.latest().params['filename'], + assert self.node.logs.count() == nlog + 1 + assert self.node.logs.latest().action == \ + f'{self.short_name}_{action}' + assert self.node.logs.latest().params['filename'] == \ filename - ) def test_set_folder(self): dataverse = utils.create_mock_dataverse() dataset = utils.create_mock_dataset() self.node_settings.set_folder(dataverse, dataset, auth=Auth(self.user)) # Folder was set - assert_equal(self.node_settings.folder_id, dataset.id) + assert self.node_settings.folder_id == dataset.id # Log was saved last_log = self.node.logs.latest() - assert_equal(last_log.action, '{0}_dataset_linked'.format(self.short_name)) + assert last_log.action == f'{self.short_name}_dataset_linked' def test_serialize_credentials(self): credentials = self.node_settings.serialize_waterbutler_credentials() - assert_is_not_none(self.node_settings.external_account.oauth_secret) + assert self.node_settings.external_account.oauth_secret is not None expected = {'token': self.node_settings.external_account.oauth_secret} - assert_equal(credentials, expected) + assert credentials == expected def test_serialize_settings(self): settings = self.node_settings.serialize_waterbutler_settings() @@ -94,7 +89,7 @@ def test_serialize_settings(self): 'id': self.node_settings.dataset_id, 'name': self.node_settings.dataset, } - assert_equal(settings, expected) + assert settings == expected class TestUserSettings(OAuthAddonUserSettingTestSuiteMixin, unittest.TestCase): diff --git a/addons/dataverse/tests/test_serializer.py b/addons/dataverse/tests/test_serializer.py index a6b47bb5c13..fa781d6b81f 100644 --- a/addons/dataverse/tests/test_serializer.py +++ b/addons/dataverse/tests/test_serializer.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- -from nose.tools import * # noqa -import mock +from unittest import mock import pytest @@ -25,7 +23,7 @@ class TestDataverseSerializer(OAuthAddonSerializerTestSuiteMixin, OsfTestCase): required_settings_authorized = ('ownerName', ) def setUp(self): - super(TestDataverseSerializer, self).setUp() + super().setUp() self.ser = self.Serializer( user_settings=self.user_settings, node_settings=self.node_settings @@ -36,7 +34,7 @@ def setUp(self): def tearDown(self): self.mock_api.stop() - super(TestDataverseSerializer, self).tearDown() + super().tearDown() def test_serialize_acccount(self): ea = self.ExternalAccountFactory() @@ -49,6 +47,6 @@ def test_serialize_acccount(self): 'profile_url': ea.profile_url, 'nodes': [], 'host': ea.oauth_key, - 'host_url': 'https://{0}'.format(ea.oauth_key), + 'host_url': f'https://{ea.oauth_key}', } - assert_equal(self.ser.serialize_account(ea), expected) + assert self.ser.serialize_account(ea) == expected diff --git a/addons/dataverse/tests/test_utils.py b/addons/dataverse/tests/test_utils.py index 35f47278b18..2e230c9dffa 100644 --- a/addons/dataverse/tests/test_utils.py +++ b/addons/dataverse/tests/test_utils.py @@ -1,13 +1,13 @@ -from nose.tools import ( - assert_equal, assert_true, assert_false, assert_is_instance -) import pytest from dataverse import Dataverse, Dataset, DataverseFile from addons.dataverse.tests.utils import ( - create_mock_dataverse, create_mock_dataset, create_mock_draft_file, - create_mock_connection, DataverseAddonTestCase, + create_mock_dataverse, + create_mock_dataset, + create_mock_draft_file, + create_mock_connection, + DataverseAddonTestCase, ) pytestmark = pytest.mark.django_db @@ -17,45 +17,43 @@ class TestUtils(DataverseAddonTestCase): def test_mock_connection(self): mock_connection = create_mock_connection() - assert_equal(mock_connection.token, 'snowman-frosty') - assert_equal(len(mock_connection.get_dataverses()), 3) - assert_is_instance(mock_connection.get_dataverses()[0], Dataverse) - assert_equal( - mock_connection.get_dataverse(mock_connection.get_dataverses()[1].alias), - mock_connection.get_dataverses()[1], + assert mock_connection.token == 'snowman-frosty' + assert len(mock_connection.get_dataverses()) == 3 + assert isinstance(mock_connection.get_dataverses()[0], Dataverse) + assert ( + mock_connection.get_dataverse(mock_connection.get_dataverses()[1].alias) + == mock_connection.get_dataverses()[1] ) def test_mock_dataverse(self): mock_dv = create_mock_dataverse('Example 1') - assert_equal(mock_dv.title, 'Example 1') - assert_true(mock_dv.is_published) - assert_equal(mock_dv.alias, 'ALIAS1') - assert_equal(len(mock_dv.get_datasets()), 3) - assert_is_instance(mock_dv.get_datasets()[0], Dataset) - assert_equal(mock_dv.get_dataset_by_doi(mock_dv.get_datasets()[1].doi), - mock_dv.get_datasets()[1]) + assert mock_dv.title == 'Example 1' + assert mock_dv.is_published + assert mock_dv.alias == 'ALIAS1' + assert len(mock_dv.get_datasets()) == 3 + assert isinstance(mock_dv.get_datasets()[0], Dataset) + assert mock_dv.get_dataset_by_doi(mock_dv.get_datasets()[1].doi) == mock_dv.get_datasets()[1] def test_mock_dataset(self): dataset_id = 'DVN/23456' - doi = 'doi:12.3456/{0}'.format(dataset_id) + doi = f'doi:12.3456/{dataset_id}' mock_dataset = create_mock_dataset(dataset_id) - assert_equal(mock_dataset.doi, doi) - assert_equal(mock_dataset.citation, - 'Example Citation for {0}'.format(dataset_id)) - assert_equal(mock_dataset.title, 'Example ({0})'.format(dataset_id)) - assert_equal(mock_dataset.doi, doi) - assert_equal(mock_dataset.get_state(), 'DRAFT') - assert_equal(len(mock_dataset.get_files()), 1) - assert_false(mock_dataset.get_files()[0].is_published) - assert_true(mock_dataset.get_files(published=True)[0].is_published) - assert_false(mock_dataset.get_file('name.txt').is_published) - assert_true(mock_dataset.get_file('name.txt', published=True).is_published) - assert_false(mock_dataset.get_file_by_id('123').is_published) - assert_true(mock_dataset.get_file_by_id('123', published=True).is_published) + assert mock_dataset.doi == doi + assert mock_dataset.citation == f'Example Citation for {dataset_id}' + assert mock_dataset.title == f'Example ({dataset_id})' + assert mock_dataset.doi == doi + assert mock_dataset.get_state() == 'DRAFT' + assert len(mock_dataset.get_files()) == 1 + assert not mock_dataset.get_files()[0].is_published + assert mock_dataset.get_files(published=True)[0].is_published + assert not mock_dataset.get_file('name.txt').is_published + assert mock_dataset.get_file('name.txt', published=True).is_published + assert not mock_dataset.get_file_by_id('123').is_published + assert mock_dataset.get_file_by_id('123', published=True).is_published def test_mock_dvn_file(self): fid = '65432' mock_file = create_mock_draft_file(fid) - assert_equal(mock_file.name, 'file.txt') - assert_equal(mock_file.id, fid) - assert_is_instance(mock_file, DataverseFile) + assert mock_file.name == 'file.txt' + assert mock_file.id == fid + assert isinstance(mock_file, DataverseFile) diff --git a/addons/dataverse/tests/test_views.py b/addons/dataverse/tests/test_views.py index bcf7e6ef1ca..e6c1faabf4d 100644 --- a/addons/dataverse/tests/test_views.py +++ b/addons/dataverse/tests/test_views.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- -from nose.tools import (assert_false, assert_equal, assert_in, assert_true) -import mock +from unittest import mock import pytest import unittest @@ -28,19 +26,19 @@ def test_deauthorize(self): self.app.delete(url, auth=self.user.auth) self.node_settings.reload() - assert_false(self.node_settings.dataverse_alias) - assert_false(self.node_settings.dataverse) - assert_false(self.node_settings.dataset_doi) - assert_false(self.node_settings.dataset) - assert_false(self.node_settings.user_settings) + assert not self.node_settings.dataverse_alias + assert not self.node_settings.dataverse + assert not self.node_settings.dataset_doi + assert not self.node_settings.dataset + assert not self.node_settings.user_settings # Log states that node was deauthorized self.project.reload() last_log = self.project.logs.latest() - assert_equal(last_log.action, 'dataverse_node_deauthorized') + assert last_log.action == 'dataverse_node_deauthorized' log_params = last_log.params - assert_equal(log_params['node'], self.project._primary_key) - assert_equal(log_params['project'], None) + assert log_params['node'] == self.project._primary_key + assert log_params['project'] is None def test_user_config_get(self): url = api_url_for('dataverse_user_config_get') @@ -48,9 +46,9 @@ def test_user_config_get(self): res = self.app.get(url, auth=new_user.auth) result = res.json.get('result') - assert_false(result['userHasAuth']) - assert_in('hosts', result) - assert_in('create', result['urls']) + assert not result['userHasAuth'] + assert 'hosts' in result + assert 'create' in result['urls'] # userHasAuth is true with external accounts new_user.external_accounts.add(create_external_account()) @@ -58,7 +56,7 @@ def test_user_config_get(self): res = self.app.get(url, auth=self.user.auth) result = res.json.get('result') - assert_true(result['userHasAuth']) + assert result['userHasAuth'] class TestConfigViews(DataverseAddonTestCase, OAuthAddonConfigViewsTestCaseMixin, OsfTestCase): connection = create_mock_connection() @@ -66,14 +64,14 @@ class TestConfigViews(DataverseAddonTestCase, OAuthAddonConfigViewsTestCaseMixin client = DataverseProvider def setUp(self): - super(TestConfigViews, self).setUp() + super().setUp() self.mock_ser_api = mock.patch('addons.dataverse.serializer.client.connect_from_settings') self.mock_ser_api.return_value = create_mock_connection() self.mock_ser_api.start() def tearDown(self): self.mock_ser_api.stop() - super(TestConfigViews, self).tearDown() + super().tearDown() @mock.patch('addons.dataverse.views.client.connect_from_settings') def test_folder_list(self, mock_connection): @@ -82,42 +80,40 @@ def test_folder_list(self, mock_connection): url = api_url_for('dataverse_get_datasets', pid=self.project._primary_key) params = {'alias': 'ALIAS1'} - res = self.app.post_json(url, params, auth=self.user.auth) + res = self.app.post(url, json=params, auth=self.user.auth) - assert_equal(len(res.json['datasets']), 3) + assert len(res.json['datasets']) == 3 first = res.json['datasets'][0] - assert_equal(first['title'], 'Example (DVN/00001)') - assert_equal(first['doi'], 'doi:12.3456/DVN/00001') + assert first['title'] == 'Example (DVN/00001)' + assert first['doi'] == 'doi:12.3456/DVN/00001' @mock.patch('addons.dataverse.views.client.connect_from_settings') def test_set_config(self, mock_connection): mock_connection.return_value = self.connection - url = self.project.api_url_for('{0}_set_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.post_json(url, { + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config') + res = self.app.post(url, json={ 'dataverse': {'alias': 'ALIAS3'}, 'dataset': {'doi': 'doi:12.3456/DVN/00003'}, }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.project.reload() - assert_equal( - self.project.logs.latest().action, - '{0}_dataset_linked'.format(self.ADDON_SHORT_NAME) - ) - assert_equal(res.json['dataverse'], self.connection.get_dataverse('ALIAS3').title) - assert_equal(res.json['dataset'], - self.connection.get_dataverse('ALIAS3').get_dataset_by_doi('doi:12.3456/DVN/00003').title) + assert self.project.logs.latest().action == \ + f'{self.ADDON_SHORT_NAME}_dataset_linked' + assert res.json['dataverse'] == self.connection.get_dataverse('ALIAS3').title + assert res.json['dataset'] == \ + self.connection.get_dataverse('ALIAS3').get_dataset_by_doi('doi:12.3456/DVN/00003').title def test_get_config(self): - url = self.project.api_url_for('{0}_get_config'.format(self.ADDON_SHORT_NAME)) + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config') res = self.app.get(url, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) - assert_in('result', res.json) + assert res.status_code == http_status.HTTP_200_OK + assert 'result' in res.json serialized = self.Serializer().serialize_settings( self.node_settings, self.user, ) - assert_equal(serialized, res.json['result']) + assert serialized == res.json['result'] @mock.patch('addons.dataverse.views.client.connect_from_settings') def test_set_config_no_dataset(self, mock_connection): @@ -132,19 +128,18 @@ def test_set_config_no_dataset(self, mock_connection): } # Select a different dataset - res = self.app.post_json(url, params, auth=self.user.auth, - expect_errors=True) + res = self.app.post(url, json=params, auth=self.user.auth) self.node_settings.reload() # Old settings did not change - assert_equal(res.status_code, http_status.HTTP_400_BAD_REQUEST) - assert_equal(self.node_settings.dataverse_alias, 'ALIAS2') - assert_equal(self.node_settings.dataset, 'Example (DVN/00001)') - assert_equal(self.node_settings.dataset_doi, 'doi:12.3456/DVN/00001') + assert res.status_code == http_status.HTTP_400_BAD_REQUEST + assert self.node_settings.dataverse_alias == 'ALIAS2' + assert self.node_settings.dataset == 'Example (DVN/00001)' + assert self.node_settings.dataset_doi == 'doi:12.3456/DVN/00001' # Nothing was logged self.project.reload() - assert_equal(self.project.logs.count(), num_old_logs) + assert self.project.logs.count() == num_old_logs class TestHgridViews(DataverseAddonTestCase, OsfTestCase, unittest.TestCase): @@ -175,16 +170,16 @@ def test_dataverse_root_published(self, mock_files, mock_connection, mock_text): # Contributor can select between states, current state is correct res = self.app.get(url, auth=self.user.auth) - assert_true(res.json[0]['permissions']['edit']) - assert_true(res.json[0]['hasPublishedFiles']) - assert_equal(res.json[0]['version'], 'latest-published') + assert res.json[0]['permissions']['edit'] + assert res.json[0]['hasPublishedFiles'] + assert res.json[0]['version'] == 'latest-published' # Non-contributor gets published version, no options user2 = AuthUserFactory() res = self.app.get(url, auth=user2.auth) - assert_false(res.json[0]['permissions']['edit']) - assert_true(res.json[0]['hasPublishedFiles']) - assert_equal(res.json[0]['version'], 'latest-published') + assert not res.json[0]['permissions']['edit'] + assert res.json[0]['hasPublishedFiles'] + assert res.json[0]['version'] == 'latest-published' @mock.patch('addons.dataverse.views.client.get_custom_publish_text') @mock.patch('addons.dataverse.views.client.connect_from_settings') @@ -212,14 +207,14 @@ def test_dataverse_root_not_published(self, mock_files, mock_connection, mock_te # Contributor gets draft, no options res = self.app.get(url, auth=self.user.auth) - assert_true(res.json[0]['permissions']['edit']) - assert_false(res.json[0]['hasPublishedFiles']) - assert_equal(res.json[0]['version'], 'latest') + assert res.json[0]['permissions']['edit'] + assert not res.json[0]['hasPublishedFiles'] + assert res.json[0]['version'] == 'latest' # Non-contributor gets nothing user2 = AuthUserFactory() res = self.app.get(url, auth=user2.auth) - assert_equal(res.json, []) + assert res.json == [] @mock.patch('addons.dataverse.views.client.connect_from_settings') @mock.patch('addons.dataverse.views.client.get_files') @@ -232,7 +227,7 @@ def test_dataverse_root_no_connection(self, mock_files, mock_connection): mock_connection.return_value = None res = self.app.get(url, auth=self.user.auth) - assert_equal(res.json, []) + assert res.json == [] def test_dataverse_root_incomplete(self): self.node_settings.dataset_doi = None @@ -242,7 +237,7 @@ def test_dataverse_root_incomplete(self): pid=self.project._primary_key) res = self.app.get(url, auth=self.user.auth) - assert_equal(res.json, []) + assert res.json == [] class TestCrudViews(DataverseAddonTestCase, OsfTestCase, unittest.TestCase): @@ -255,11 +250,11 @@ def test_dataverse_publish_dataset(self, mock_publish_dv, mock_publish_ds, mock_ url = api_url_for('dataverse_publish_dataset', pid=self.project._primary_key) - self.app.put_json(url, params={'publish_both': False}, auth=self.user.auth) + self.app.put(url, json={'publish_both': False}, auth=self.user.auth) # Only dataset was published - assert_false(mock_publish_dv.called) - assert_true(mock_publish_ds.called) + assert not mock_publish_dv.called + assert mock_publish_ds.called @mock.patch('addons.dataverse.views.client.connect_from_settings_or_401') @mock.patch('addons.dataverse.views.client.publish_dataset') @@ -269,11 +264,11 @@ def test_dataverse_publish_both(self, mock_publish_dv, mock_publish_ds, mock_con url = api_url_for('dataverse_publish_dataset', pid=self.project._primary_key) - self.app.put_json(url, params={'publish_both': True}, auth=self.user.auth) + self.app.put(url, json={'publish_both': True}, auth=self.user.auth) # Both Dataverse and dataset were published - assert_true(mock_publish_dv.called) - assert_true(mock_publish_ds.called) + assert mock_publish_dv.called + assert mock_publish_ds.called class TestDataverseRestrictions(DataverseAddonTestCase, OsfTestCase): @@ -301,6 +296,5 @@ def test_restricted_set_dataset_not_owner(self, mock_connection): 'dataverse': {'alias': 'ALIAS1'}, 'dataset': {'doi': 'doi:12.3456/DVN/00002'}, } - res = self.app.post_json(url, params, auth=self.contrib.auth, - expect_errors=True) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + res = self.app.post(url, json=params, auth=self.contrib.auth) + assert res.status_code == http_status.HTTP_403_FORBIDDEN diff --git a/addons/dataverse/tests/utils.py b/addons/dataverse/tests/utils.py index af64882cc2f..349fdc25a57 100644 --- a/addons/dataverse/tests/utils.py +++ b/addons/dataverse/tests/utils.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock from dataverse import Connection, Dataverse, Dataset, DataverseFile @@ -14,7 +14,7 @@ class DataverseAddonTestCase(OAuthAddonTestCaseMixin, AddonTestCase): Provider = DataverseProvider def set_node_settings(self, settings): - super(DataverseAddonTestCase, self).set_node_settings(settings) + super().set_node_settings(settings) settings.dataverse_alias = 'ALIAS2' settings.dataverse = 'Example 2' settings.dataset_doi = 'doi:12.3456/DVN/00001' @@ -85,7 +85,7 @@ def create_mock_dataverse(title='Example Dataverse 0'): type(mock_dataverse).title = mock.PropertyMock(return_value=title) type(mock_dataverse).is_published = mock.PropertyMock(return_value=True) type(mock_dataverse).alias = mock.PropertyMock( - return_value='ALIAS{}'.format(title[-1]) + return_value=f'ALIAS{title[-1]}' ) mock_dataverse.get_datasets.return_value = [ @@ -110,9 +110,9 @@ def _get_dataset_by_doi(doi, timeout=None): def create_mock_dataset(id='DVN/12345'): mock_dataset = mock.create_autospec(Dataset) - mock_dataset.citation = 'Example Citation for {0}'.format(id) - mock_dataset.title = 'Example ({0})'.format(id) - mock_dataset.doi = 'doi:12.3456/{0}'.format(id) + mock_dataset.citation = f'Example Citation for {id}' + mock_dataset.title = f'Example ({id})' + mock_dataset.doi = f'doi:12.3456/{id}' mock_dataset.id = '18' mock_dataset.get_state.return_value = 'DRAFT' @@ -150,16 +150,16 @@ def create_mock_published_file(id='54321'): mock_responses = { 'contents': { - u'kind': u'item', - u'name': u'file.txt', - u'ext': u'.txt', - u'file_id': u'54321', - u'urls': {u'download': u'/project/xxxxx/dataverse/file/54321/download/', - u'delete': u'/api/v1/project/xxxxx/dataverse/file/54321/', - u'view': u'/project/xxxxx/dataverse/file/54321/'}, - u'permissions': {u'edit': False, u'view': True}, - u'addon': u'dataverse', - u'hasPublishedFiles': True, - u'state': 'published', + 'kind': 'item', + 'name': 'file.txt', + 'ext': '.txt', + 'file_id': '54321', + 'urls': {'download': '/project/xxxxx/dataverse/file/54321/download/', + 'delete': '/api/v1/project/xxxxx/dataverse/file/54321/', + 'view': '/project/xxxxx/dataverse/file/54321/'}, + 'permissions': {'edit': False, 'view': True}, + 'addon': 'dataverse', + 'hasPublishedFiles': True, + 'state': 'published', } } diff --git a/addons/dataverse/views.py b/addons/dataverse/views.py index a3d5e24834f..9b549e6de9b 100644 --- a/addons/dataverse/views.py +++ b/addons/dataverse/views.py @@ -1,5 +1,4 @@ """Views for the node settings page.""" -# -*- coding: utf-8 -*- from rest_framework import status as http_status from django.utils import timezone @@ -312,7 +311,7 @@ def dataverse_get_widget_contents(node_addon, **kwargs): return {'data': data}, http_status.HTTP_400_BAD_REQUEST dataverse_host = node_addon.external_account.oauth_key - dataverse_url = 'http://{0}/dataverse/{1}'.format(dataverse_host, alias) + dataverse_url = f'http://{dataverse_host}/dataverse/{alias}' dataset_url = 'https://doi.org/' + doi data.update({ diff --git a/addons/dropbox/__init__.py b/addons/dropbox/__init__.py index a8685d3c977..e69de29bb2d 100644 --- a/addons/dropbox/__init__.py +++ b/addons/dropbox/__init__.py @@ -1 +0,0 @@ -default_app_config = 'addons.dropbox.apps.DropboxAddonAppConfig' diff --git a/addons/dropbox/apps.py b/addons/dropbox/apps.py index 9b8c26bafe8..07ec04355d2 100644 --- a/addons/dropbox/apps.py +++ b/addons/dropbox/apps.py @@ -4,8 +4,10 @@ dropbox_root_folder = generic_root_folder('dropbox') + class DropboxAddonAppConfig(BaseAddonAppConfig): + default = True name = 'addons.dropbox' label = 'addons_dropbox' full_name = 'Dropbox' diff --git a/addons/dropbox/migrations/0001_initial.py b/addons/dropbox/migrations/0001_initial.py index f62db239046..43f59b49b24 100644 --- a/addons/dropbox/migrations/0001_initial.py +++ b/addons/dropbox/migrations/0001_initial.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals import addons.base.models from django.db import migrations, models diff --git a/addons/dropbox/migrations/0002_auto_20220817_1915.py b/addons/dropbox/migrations/0002_auto_20220817_1915.py index 1de073e684c..bd2981d90e8 100644 --- a/addons/dropbox/migrations/0002_auto_20220817_1915.py +++ b/addons/dropbox/migrations/0002_auto_20220817_1915.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.29 on 2022-08-17 19:15 -from __future__ import unicode_literals from django.conf import settings from django.db import migrations, models diff --git a/addons/dropbox/migrations/0003_alter_nodesettings_external_account_and_more.py b/addons/dropbox/migrations/0003_alter_nodesettings_external_account_and_more.py new file mode 100644 index 00000000000..e1ed18415e5 --- /dev/null +++ b/addons/dropbox/migrations/0003_alter_nodesettings_external_account_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.13 on 2024-07-15 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0021_preprint_custom_publication_citation'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('addons_dropbox', '0002_auto_20220817_1915'), + ] + + operations = [ + migrations.AlterField( + model_name='nodesettings', + name='external_account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.externalaccount'), + ), + migrations.AlterField( + model_name='nodesettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_node_settings', to='osf.abstractnode'), + ), + migrations.AlterField( + model_name='usersettings', + name='owner', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='%(app_label)s_user_settings', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/addons/dropbox/models.py b/addons/dropbox/models.py index 05a7e6797ad..d28d761dcd9 100644 --- a/addons/dropbox/models.py +++ b/addons/dropbox/models.py @@ -5,7 +5,7 @@ from addons.base.models import (BaseOAuthNodeSettings, BaseOAuthUserSettings, BaseStorageAddon) from django.db import models -from dropbox.dropbox import Dropbox +from dropbox import Dropbox from dropbox.exceptions import ApiError, DropboxException from dropbox.files import FolderMetadata from furl import furl @@ -55,7 +55,7 @@ class Provider(ExternalProvider): def auth_url(self): # Dropbox requires explicitly requesting refresh_tokens via `token_access_type` # https://developers.dropbox.com/oauth-guide#implementing-oauth - url = super(Provider, self).auth_url + url = super().auth_url return furl(url).add({'token_access_type': 'offline'}).url def handle_callback(self, response): @@ -122,7 +122,7 @@ def folder_path(self): @property def display_name(self): - return '{0}: {1}'.format(self.config.full_name, self.folder) + return f'{self.config.full_name}: {self.folder}' def fetch_access_token(self): return self.api.fetch_access_token() @@ -140,9 +140,7 @@ def get_folders(self, **kwargs): 'kind': 'folder', 'name': '/ (Full Dropbox)', 'urls': { - 'folders': api_v2_url('nodes/{}/addons/dropbox/folders/'.format(self.owner._id), - params={'id': '/'} - ) + 'folders': api_v2_url(f'nodes/{self.owner._id}/addons/dropbox/folders/', params={'id': '/'}) } }] @@ -171,8 +169,8 @@ def get_folders(self, **kwargs): 'name': item.path_display.split('/')[-1], 'path': item.path_display, 'urls': { - 'folders': api_v2_url('nodes/{}/addons/dropbox/folders/'.format(self.owner._id), - params={'id': item.path_display} + 'folders': api_v2_url( + f'nodes/{self.owner._id}/addons/dropbox/folders/', params={'id': item.path_display} ) } } @@ -207,12 +205,13 @@ def serialize_waterbutler_settings(self): return {'folder': self.folder} def create_waterbutler_log(self, auth, action, metadata): - url = self.owner.web_url_for('addon_view_or_download_file', + url = self.owner.web_url_for( + 'addon_view_or_download_file', path=metadata['path'].strip('/'), provider='dropbox' ) self.owner.add_log( - 'dropbox_{0}'.format(action), + f'dropbox_{action}', auth=auth, params={ 'project': self.owner.parent_id, @@ -227,7 +226,7 @@ def create_waterbutler_log(self, auth, action, metadata): ) def __repr__(self): - return u'