From bd7d834b77033f113ac7922902b7fda49bf2e7c0 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 5 Apr 2024 10:43:53 -0400 Subject: [PATCH 01/21] Remove runsv, simplify Dockerfile --- Dockerfile | 30 +++----------------------- docker/{init.bash => entrypoint.sh} | 17 ++++++++------- docker/run_celery_beat.bash | 12 ----------- docker/run_celery_kobocat.bash | 20 ----------------- docker/run_celery_kpi.bash | 20 ----------------- docker/run_celery_low_priority.bash | 20 ----------------- docker/run_tests.bash | 7 ------ docker/run_uwsgi.bash | 15 ------------- docker/setup_pydev.bash | 33 ----------------------------- docker/uwsgi.ini | 20 ++++++++--------- 10 files changed, 22 insertions(+), 172 deletions(-) rename docker/{init.bash => entrypoint.sh} (89%) delete mode 100755 docker/run_celery_beat.bash delete mode 100755 docker/run_celery_kobocat.bash delete mode 100755 docker/run_celery_kpi.bash delete mode 100755 docker/run_celery_low_priority.bash delete mode 100755 docker/run_tests.bash delete mode 100755 docker/run_uwsgi.bash delete mode 100755 docker/setup_pydev.bash diff --git a/Dockerfile b/Dockerfile index 2038b4fb03..bfaf1b5665 100644 --- a/Dockerfile +++ b/Dockerfile @@ -29,8 +29,6 @@ ENV KPI_LOGS_DIR=/srv/logs \ TMP_DIR=/srv/tmp \ UWSGI_USER=kobo \ UWSGI_GROUP=kobo \ - SERVICES_DIR=/etc/service \ - CELERY_PID_DIR=/var/run/celery \ INIT_PATH=/srv/init ########################################## @@ -40,14 +38,7 @@ ENV KPI_LOGS_DIR=/srv/logs \ RUN mkdir -p "${NGINX_STATIC_DIR}" && \ mkdir -p "${KPI_SRC_DIR}" && \ mkdir -p "${KPI_NODE_PATH}" && \ - mkdir -p "${TMP_DIR}" && \ - mkdir -p ${CELERY_PID_DIR} && \ - mkdir -p ${SERVICES_DIR}/uwsgi && \ - mkdir -p ${SERVICES_DIR}/celery_kpi && \ - mkdir -p ${SERVICES_DIR}/celery_kobocat && \ - mkdir -p ${SERVICES_DIR}/celery_low_priority && \ - mkdir -p ${SERVICES_DIR}/celery_beat && \ - mkdir -p "${INIT_PATH}" + mkdir -p "${TMP_DIR}" ########################################## # Install `apt` packages. # @@ -78,7 +69,6 @@ RUN apt-get -qq update && \ postgresql-client \ procps \ rsync \ - runit-init \ vim-tiny \ wait-for-it && \ apt-get clean && \ @@ -163,24 +153,10 @@ RUN echo "export PATH=${PATH}" >> /etc/profile && \ echo 'source /etc/profile' >> /root/.bashrc && \ echo 'source /etc/profile' >> /home/${UWSGI_USER}/.bashrc - -# Remove getty* services to avoid errors of absent tty at sv start-up -RUN rm -rf /etc/runit/runsvdir/default/getty-tty* - -# Create symlinks for runsv services -RUN ln -s "${KPI_SRC_DIR}/docker/run_uwsgi.bash" "${SERVICES_DIR}/uwsgi/run" && \ - ln -s "${KPI_SRC_DIR}/docker/run_celery_kpi.bash" "${SERVICES_DIR}/celery_kpi/run" && \ - ln -s "${KPI_SRC_DIR}/docker/run_celery_kobocat.bash" "${SERVICES_DIR}/celery_kobocat/run" && \ - ln -s "${KPI_SRC_DIR}/docker/run_celery_low_priority.bash" "${SERVICES_DIR}/celery_low_priority/run" && \ - ln -s "${KPI_SRC_DIR}/docker/run_celery_beat.bash" "${SERVICES_DIR}/celery_beat/run" - - # Add/Restore `UWSGI_USER`'s permissions # chown of `${TMP_DIR}/.npm` is a hack needed for kobo-install-based staging deployments; # see internal discussion at https://chat.kobotoolbox.org/#narrow/stream/4-Kobo-Dev/topic/Unu.2C.20du.2C.20tri.2C.20kvar.20deployments/near/322075 -RUN chown -R ":${UWSGI_GROUP}" ${CELERY_PID_DIR} && \ - chmod g+w ${CELERY_PID_DIR} && \ - chown -R "${UWSGI_USER}:${UWSGI_GROUP}" ${KPI_SRC_DIR}/emails/ && \ +RUN chown -R "${UWSGI_USER}:${UWSGI_GROUP}" ${KPI_SRC_DIR}/emails/ && \ chown -R "${UWSGI_USER}:${UWSGI_GROUP}" ${KPI_LOGS_DIR} && \ chown -R "${UWSGI_USER}:${UWSGI_GROUP}" ${TMP_DIR} && \ chown -R root:root "${TMP_DIR}/.npm" @@ -188,4 +164,4 @@ RUN chown -R ":${UWSGI_GROUP}" ${CELERY_PID_DIR} && \ EXPOSE 8000 -CMD ["/bin/bash", "-c", "exec ${KPI_SRC_DIR}/docker/init.bash"] +CMD ["/bin/bash", "-c", "exec ${KPI_SRC_DIR}/docker/entrypoint.sh"] diff --git a/docker/init.bash b/docker/entrypoint.sh similarity index 89% rename from docker/init.bash rename to docker/entrypoint.sh index bbab9a1a00..ed7b0a3079 100755 --- a/docker/init.bash +++ b/docker/entrypoint.sh @@ -14,7 +14,7 @@ if [[ -z $DATABASE_URL ]]; then fi # Handle Python dependencies BEFORE attempting any `manage.py` commands -KPI_WEB_SERVER="${KPI_WEB_SERVER:-uWSGI}" +KPI_WEB_SERVER="${UWSGI:-uWSGI}" if [[ "${KPI_WEB_SERVER,,}" == 'uwsgi' ]]; then # `diff` returns exit code 1 if it finds a difference between the files if ! diff -q "${KPI_SRC_DIR}/dependencies/pip/requirements.txt" "${TMP_DIR}/pip_dependencies.txt" @@ -77,12 +77,6 @@ if [[ ! -d "${KPI_SRC_DIR}/locale" ]] || [[ -z "$(ls -A ${KPI_SRC_DIR}/locale)" python manage.py compilemessages fi -rm -rf /etc/profile.d/pydev_debugger.bash.sh -if [[ -d /srv/pydev_orig && -n "${KPI_PATH_FROM_ECLIPSE_TO_PYTHON_PAIRS}" ]]; then - echo 'Enabling PyDev remote debugging.' - "${KPI_SRC_DIR}/docker/setup_pydev.bash" -fi - echo 'Cleaning up Celery PIDs…' rm -rf /tmp/celery*.pid @@ -96,4 +90,11 @@ chown -R "${UWSGI_USER}:${UWSGI_GROUP}" "${KPI_MEDIA_DIR}" echo 'KPI initialization completed.' -exec /usr/bin/runsvdir "${SERVICES_DIR}" +cd "${KPI_SRC_DIR}" +if [[ "${UWSGI,,}" == 'uwsgi' ]]; then + echo "Running \`kpi\` container with uWSGI application server." + $(command -v uwsgi) --ini ${KPI_SRC_DIR}/docker/uwsgi.ini +else + echo "Running \`kpi\` container with \`runserver_plus\` debugging application server." + python manage.py runserver_plus 0:8000 +fi diff --git a/docker/run_celery_beat.bash b/docker/run_celery_beat.bash deleted file mode 100755 index 27fb36b8d3..0000000000 --- a/docker/run_celery_beat.bash +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash -set -e -source /etc/profile - -# Run the main Celery worker (will not process `sync_kobocat_xforms` jobs). -cd "${KPI_SRC_DIR}" -exec celery -A kobo beat --loglevel=info \ - --logfile=${KPI_LOGS_DIR}/celery_beat.log \ - --pidfile=/tmp/celery_beat.pid \ - --scheduler django_celery_beat.schedulers:DatabaseScheduler \ - --uid=${UWSGI_USER} \ - --gid=${UWSGI_GROUP} diff --git a/docker/run_celery_kobocat.bash b/docker/run_celery_kobocat.bash deleted file mode 100755 index 0b6d684164..0000000000 --- a/docker/run_celery_kobocat.bash +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash -set -e -source /etc/profile - -# Run the main Celery worker (will NOT process low-priority jobs) - -cd "${KPI_SRC_DIR}" - -AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" -AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" - -exec celery -A kobo worker --loglevel=info \ - --hostname=kobocat_main_worker@%h \ - --logfile=${KPI_LOGS_DIR}/celery_kobocat.log \ - --pidfile=/tmp/celery_kobocat.pid \ - --queues=kobocat_queue \ - --exclude-queues=kpi_low_priority_queue,kpi_queue \ - --uid=${UWSGI_USER} \ - --gid=${UWSGI_GROUP} \ - --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} diff --git a/docker/run_celery_kpi.bash b/docker/run_celery_kpi.bash deleted file mode 100755 index 2c059bf7d8..0000000000 --- a/docker/run_celery_kpi.bash +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash -set -e -source /etc/profile - -# Run the main Celery worker (will NOT process low-priority jobs) - -cd "${KPI_SRC_DIR}" - -AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" -AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" - -exec celery -A kobo worker --loglevel=info \ - --hostname=kpi_main_worker@%h \ - --logfile=${KPI_LOGS_DIR}/celery_kpi.log \ - --pidfile=/tmp/celery_kpi.pid \ - --queues=kpi_queue \ - --exclude-queues=kpi_low_priority_queue,kobocat_queue \ - --uid=${UWSGI_USER} \ - --gid=${UWSGI_GROUP} \ - --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} diff --git a/docker/run_celery_low_priority.bash b/docker/run_celery_low_priority.bash deleted file mode 100755 index bf8ab2e01f..0000000000 --- a/docker/run_celery_low_priority.bash +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash -set -e -source /etc/profile - -# Run the Celery worker for low-priority jobs ONLY - -cd "${KPI_SRC_DIR}" - -AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" -AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" - -exec celery -A kobo worker --loglevel=info \ - --hostname=kpi_main_worker@%h \ - --logfile=${KPI_LOGS_DIR}/celery_low_priority.log \ - --pidfile=/tmp/celery_low_priority.pid \ - --queues=kpi_low_priority_queue \ - --exclude-queues=kpi_queue,kobocat_queue \ - --uid=${UWSGI_USER} \ - --gid=${UWSGI_GROUP} \ - --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} diff --git a/docker/run_tests.bash b/docker/run_tests.bash deleted file mode 100755 index e03b24ceb8..0000000000 --- a/docker/run_tests.bash +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -set -e - -source /etc/profile - -pytest -npm run test diff --git a/docker/run_uwsgi.bash b/docker/run_uwsgi.bash deleted file mode 100755 index 13dc9dc9ef..0000000000 --- a/docker/run_uwsgi.bash +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash - -source /etc/profile - -KPI_WEB_SERVER="${KPI_WEB_SERVER:-uWSGI}" -UWSGI_COMMAND="$(command -v uwsgi) --ini ${KPI_SRC_DIR}/docker/uwsgi.ini" - -cd "${KPI_SRC_DIR}" -if [[ "${KPI_WEB_SERVER,,}" == 'uwsgi' ]]; then - echo "Running \`kpi\` container with uWSGI application server." - exec ${UWSGI_COMMAND} -else - echo "Running \`kpi\` container with \`runserver_plus\` debugging application server." - exec python manage.py runserver_plus 0:8000 -fi diff --git a/docker/setup_pydev.bash b/docker/setup_pydev.bash deleted file mode 100755 index 482517ba08..0000000000 --- a/docker/setup_pydev.bash +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/env bash -set -e - -if [[ ! -d /srv/pydev_orig ]]; then - echo 'Directory `/srv/pydev_orig` must exist to use PyDev debugger (see `kobo-docker/docker-compose.yml`).' - exit 1 -fi - -cp -a /srv/pydev_orig /srv/pydev - -if [[ -z "${KPI_PATH_FROM_ECLIPSE_TO_PYTHON_PAIRS}" ]]; then - echo '`KPI_PATH_FROM_ECLIPSE_TO_PYTHON_PAIRS` must be set to use the PyDev debugger (see `kobo-docker/envfiles/kpi.txt`).' - exit 1 -fi - -echo 'Setting up PyDev remote debugger path mappings.' - -# Set up the `PATHS_FROM_ECLIPSE_TO_PYTHON` variable from the environment per -# https://github.com/fabioz/PyDev.Debugger/blob/master/pydevd_file_utils.py. -find_string='PATHS_FROM_ECLIPSE_TO_PYTHON = []' -replace_string="\ -import os\n\ -path_map_pair_strings = os.environ['KPI_PATH_FROM_ECLIPSE_TO_PYTHON_PAIRS'].split('|')\n\ -PATHS_FROM_ECLIPSE_TO_PYTHON = [tuple([pair_element.strip() for pair_element in pair_string.split('->')]) for pair_string in path_map_pair_strings]\n\ -" - -escaped_find_sting="$(echo "${find_string}" | sed -e 's/[]\/$*.^|[]/\\&/g')" -escaped_replace_string=$(echo "${replace_string}" | sed -e '/\\n/b; s/[]\/$*.^|[]/\\&/g') - -sed -i "s/${escaped_find_sting}/${escaped_replace_string}/" /srv/pydev/pydevd_file_utils.py - -echo 'Adding `PYTHONPATH` modifications to profile.' -echo 'export PYTHONPATH=${PYTHONPATH}:/srv/pydev' > /etc/profile.d/pydev_debugger.bash.sh diff --git a/docker/uwsgi.ini b/docker/uwsgi.ini index 5f2e139f54..6ab4bae557 100644 --- a/docker/uwsgi.ini +++ b/docker/uwsgi.ini @@ -13,23 +13,23 @@ mount = $(KPI_PREFIX)=$(KPI_SRC_DIR)/kobo/wsgi.py # process related settings master = true -harakiri = $(KPI_UWSGI_HARAKIRI) -worker-reload-mercy = $(KPI_UWSGI_WORKER_RELOAD_MERCY) +harakiri = $(UWSGI_HARAKIRI) +worker-reload-mercy = $(UWSGI_WORKER_RELOAD_MERCY) # monitoring (use with `uwsgitop :1717`, for example) stats = :1717 memory-report = true # Overrideable default of 2 uWSGI processes. -if-env = KPI_UWSGI_WORKERS_COUNT +if-env = UWSGI_WORKERS_COUNT workers = %(_) endif = -if-not-env = KPI_UWSGI_WORKERS_COUNT +if-not-env = UWSGI_WORKERS_COUNT workers = 2 endif = # activate cheaper mode -if-env = KPI_UWSGI_CHEAPER_WORKERS_COUNT +if-env = UWSGI_CHEAPER_WORKERS_COUNT cheaper-algo = busyness cheaper = %(_) cheaper-initial = %(_) @@ -41,23 +41,23 @@ cheaper-busyness-multiplier = 20 endif = # stop spawning new workers if total memory consumption grows too large -if-env = KPI_UWSGI_CHEAPER_RSS_LIMIT_SOFT +if-env = UWSGI_CHEAPER_RSS_LIMIT_SOFT cheaper-rss-limit-soft = %(_) endif = -if-not-env = KPI_UWSGI_CHEAPER_RSS_LIMIT_SOFT +if-not-env = UWSGI_CHEAPER_RSS_LIMIT_SOFT cheaper-rss-limit-soft = %(2 * 1024 * 1024 * 1024) endif = # respawn processes after serving KPI_UWSGI_MAX_REQUESTS requests (default 5000) -if-env = KPI_UWSGI_MAX_REQUESTS +if-env = UWSGI_MAX_REQUESTS max-requests = %(_) endif = # respawn workers when their memory consumption grows too large -if-env = KPI_UWSGI_RELOAD_ON_RSS_MB +if-env = UWSGI_RELOAD_ON_RSS_MB reload-on-rss = %(_) endif = -if-not-env = KPI_UWSGI_RELOAD_ON_RSS_MB +if-not-env = UWSGI_RELOAD_ON_RSS_MB reload-on-rss = 512 endif = From aa0e2fd71385ca36b63360d2be2b93c9a3f7f7e4 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 5 Apr 2024 10:44:46 -0400 Subject: [PATCH 02/21] Use env vars SENTRY_DSN and ENKETO_API_KEY, instead of RAVEN_DSN and ENKETO_API_TOKEN --- kobo/apps/openrosa/libs/utils/viewer_tools.py | 2 +- kobo/settings/base.py | 12 ++++-------- kpi/deployment_backends/kobocat_backend.py | 2 +- kpi/tasks.py | 2 +- kpi/views/v2/asset_snapshot.py | 2 +- kpi/views/v2/data.py | 2 +- 6 files changed, 9 insertions(+), 13 deletions(-) diff --git a/kobo/apps/openrosa/libs/utils/viewer_tools.py b/kobo/apps/openrosa/libs/utils/viewer_tools.py index 0125001137..16cda45572 100644 --- a/kobo/apps/openrosa/libs/utils/viewer_tools.py +++ b/kobo/apps/openrosa/libs/utils/viewer_tools.py @@ -144,7 +144,7 @@ def enketo_url( url = f'{url}/view' req = requests.post( - url, data=values, auth=(settings.ENKETO_API_TOKEN, ''), verify=False + url, data=values, auth=(settings.ENKETO_API_KEY, ''), verify=False ) if req.status_code in [200, 201]: diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 0210ab4085..0bcd600c6d 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -977,11 +977,7 @@ def __init__(self, *args, **kwargs): DEFAULT_SUBMISSIONS_COUNT_NUMBER_OF_DAYS = 31 GOOGLE_ANALYTICS_TOKEN = os.environ.get('GOOGLE_ANALYTICS_TOKEN') SENTRY_JS_DSN = None -RAVEN_JS_DSN = env.str('RAVEN_JS_DSN', None) -if SENTRY_JS_DSN_URL := env.url( - 'SENTRY_JS_DSN', - default=RAVEN_JS_DSN - ): +if SENTRY_JS_DSN_URL := env.url('SENTRY_JS_DSN', default=None): SENTRY_JS_DSN = SENTRY_JS_DSN_URL.geturl() # replace this with the pointer to the kobocat server, if it exists @@ -1047,7 +1043,7 @@ def dj_stripe_request_callback_method(): ENKETO_INTERNAL_URL = os.environ.get('ENKETO_INTERNAL_URL', ENKETO_URL) ENKETO_INTERNAL_URL = ENKETO_INTERNAL_URL.rstrip('/') # Remove any trailing slashes -ENKETO_API_TOKEN = os.environ.get('ENKETO_API_TOKEN', 'enketorules') +ENKETO_API_KEY = os.environ.get('ENKETO_API_KEY', 'enketorules') # http://apidocs.enketo.org/v2/ ENKETO_SURVEY_ENDPOINT = 'api/v2/survey/all' ENKETO_PREVIEW_ENDPOINT = 'api/v2/survey/preview/iframe' @@ -1197,7 +1193,7 @@ def dj_stripe_request_callback_method(): SYNC_KOBOCAT_PERMISSIONS = ( os.environ.get('SYNC_KOBOCAT_PERMISSIONS', 'True') == 'True') -CELERY_BROKER_URL = os.environ.get('KPI_BROKER_URL', 'redis://localhost:6379/1') +CELERY_BROKER_URL = os.environ.get('CELERY_BROKER_URL', 'redis://localhost:6379/1') CELERY_RESULT_BACKEND = CELERY_BROKER_URL # Increase limits for long-running tasks @@ -1379,7 +1375,7 @@ def dj_stripe_request_callback_method(): ################################ # Sentry settings # ################################ -sentry_dsn = env.str('SENTRY_DSN', env.str('RAVEN_DSN', None)) +sentry_dsn = env.str('SENTRY_DSN', None) if sentry_dsn: import sentry_sdk from sentry_sdk.integrations.celery import CeleryIntegration diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index e7928f3e07..67856ded21 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -685,7 +685,7 @@ def get_enketo_survey_links(self): response = requests.post( f'{settings.ENKETO_URL}/{settings.ENKETO_SURVEY_ENDPOINT}', # bare tuple implies basic auth - auth=(settings.ENKETO_API_TOKEN, ''), + auth=(settings.ENKETO_API_KEY, ''), data=data ) response.raise_for_status() diff --git a/kpi/tasks.py b/kpi/tasks.py index f98c973deb..fb8be72863 100644 --- a/kpi/tasks.py +++ b/kpi/tasks.py @@ -90,7 +90,7 @@ def enketo_flush_cached_preview(server_url, form_id): response = requests.delete( f'{settings.ENKETO_URL}/{settings.ENKETO_FLUSH_CACHE_ENDPOINT}', # bare tuple implies basic auth - auth=(settings.ENKETO_API_TOKEN, ''), + auth=(settings.ENKETO_API_KEY, ''), data=dict(server_url=server_url, form_id=form_id), ) response.raise_for_status() diff --git a/kpi/views/v2/asset_snapshot.py b/kpi/views/v2/asset_snapshot.py index b92aa2728d..d3036c7031 100644 --- a/kpi/views/v2/asset_snapshot.py +++ b/kpi/views/v2/asset_snapshot.py @@ -187,7 +187,7 @@ def preview(self, request, *args, **kwargs): response = requests.post( f'{settings.ENKETO_URL}/{settings.ENKETO_PREVIEW_ENDPOINT}', # bare tuple implies basic auth - auth=(settings.ENKETO_API_TOKEN, ''), + auth=(settings.ENKETO_API_KEY, ''), data=data ) response.raise_for_status() diff --git a/kpi/views/v2/data.py b/kpi/views/v2/data.py index 30a65c32da..402f541d09 100644 --- a/kpi/views/v2/data.py +++ b/kpi/views/v2/data.py @@ -758,7 +758,7 @@ def _get_enketo_link( response = requests.post( f'{settings.ENKETO_URL}/{enketo_endpoint}', # bare tuple implies basic auth - auth=(settings.ENKETO_API_TOKEN, ''), + auth=(settings.ENKETO_API_KEY, ''), data=data ) if response.status_code != status.HTTP_201_CREATED: From 91564df7e4284746b2b44d8cfa79f652475854fe Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 5 Apr 2024 12:52:44 -0400 Subject: [PATCH 03/21] Update/Create entrypoints for celery workers --- Dockerfile | 2 +- docker/entrypoint.sh | 3 +-- docker/entrypoint_celery_beat.bash | 13 ++++++++++++ docker/entrypoint_celery_kobocat_worker.bash | 20 +++++++++++++++++++ ...ypoint_celery_kpi_low_priority_worker.bash | 20 +++++++++++++++++++ docker/entrypoint_celery_kpi_worker.bash | 20 +++++++++++++++++++ 6 files changed, 75 insertions(+), 3 deletions(-) create mode 100755 docker/entrypoint_celery_beat.bash create mode 100755 docker/entrypoint_celery_kobocat_worker.bash create mode 100755 docker/entrypoint_celery_kpi_low_priority_worker.bash create mode 100755 docker/entrypoint_celery_kpi_worker.bash diff --git a/Dockerfile b/Dockerfile index bfaf1b5665..92fa432353 100644 --- a/Dockerfile +++ b/Dockerfile @@ -164,4 +164,4 @@ RUN chown -R "${UWSGI_USER}:${UWSGI_GROUP}" ${KPI_SRC_DIR}/emails/ && \ EXPOSE 8000 -CMD ["/bin/bash", "-c", "exec ${KPI_SRC_DIR}/docker/entrypoint.sh"] +CMD ["/bin/bash", "docker/entrypoint.sh"] diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index ed7b0a3079..33acad6027 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -1,6 +1,5 @@ #!/bin/bash set -e - source /etc/profile echo 'KPI initializing…' @@ -80,7 +79,7 @@ fi echo 'Cleaning up Celery PIDs…' rm -rf /tmp/celery*.pid -echo 'Restore permissions on Celery logs folder' +echo 'Restore permissions on logs folder' chown -R "${UWSGI_USER}:${UWSGI_GROUP}" "${KPI_LOGS_DIR}" # This can take a while when starting a container with lots of media files. diff --git a/docker/entrypoint_celery_beat.bash b/docker/entrypoint_celery_beat.bash new file mode 100755 index 0000000000..1b6d0829d6 --- /dev/null +++ b/docker/entrypoint_celery_beat.bash @@ -0,0 +1,13 @@ +#!/bin/bash +set -e +source /etc/profile + +# Run the main Celery worker (will not process `sync_kobocat_xforms` jobs). +cd "${KPI_SRC_DIR}" + +exec celery -A kobo beat --loglevel=info \ + --logfile=${KPI_LOGS_DIR}/celery_beat.log \ + --pidfile=/tmp/celery_beat.pid \ + --scheduler django_celery_beat.schedulers:DatabaseScheduler \ + --uid=${UWSGI_USER} \ + --gid=${UWSGI_GROUP} diff --git a/docker/entrypoint_celery_kobocat_worker.bash b/docker/entrypoint_celery_kobocat_worker.bash new file mode 100755 index 0000000000..85ca74f2a2 --- /dev/null +++ b/docker/entrypoint_celery_kobocat_worker.bash @@ -0,0 +1,20 @@ +#!/bin/bash +set -e +source /etc/profile + +# Run the main Celery worker (will NOT process low-priority jobs) + +cd "${KPI_SRC_DIR}" + +AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" +AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" + +exec celery -A kobo worker --loglevel=info \ + --hostname=kobocat_main_worker@%h \ + --logfile=${KPI_LOGS_DIR}/celery_kobocat_worker.log \ + --pidfile=/tmp/celery_kobocat_worker.pid \ + --queues=kobocat_queue \ + --exclude-queues=kpi_low_priority_queue,kpi_queue \ + --uid=${UWSGI_USER} \ + --gid=${UWSGI_GROUP} \ + --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} diff --git a/docker/entrypoint_celery_kpi_low_priority_worker.bash b/docker/entrypoint_celery_kpi_low_priority_worker.bash new file mode 100755 index 0000000000..5008fde4dd --- /dev/null +++ b/docker/entrypoint_celery_kpi_low_priority_worker.bash @@ -0,0 +1,20 @@ +#!/bin/bash +set -e +source /etc/profile + +# Run the Celery worker for low-priority jobs ONLY + +cd "${KPI_SRC_DIR}" + +AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" +AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" + +exec celery -A kobo worker --loglevel=info \ + --hostname=kpi_main_worker@%h \ + --logfile=${KPI_LOGS_DIR}/celery_kpi_low_priority_worker.log \ + --pidfile=/tmp/celery_kpi_low_priority_worker.pid \ + --queues=kpi_low_priority_queue \ + --exclude-queues=kpi_queue,kobocat_queue \ + --uid=${UWSGI_USER} \ + --gid=${UWSGI_GROUP} \ + --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} diff --git a/docker/entrypoint_celery_kpi_worker.bash b/docker/entrypoint_celery_kpi_worker.bash new file mode 100755 index 0000000000..d44d6a8d6b --- /dev/null +++ b/docker/entrypoint_celery_kpi_worker.bash @@ -0,0 +1,20 @@ +#!/bin/bash +set -e +source /etc/profile + +# Run the main Celery worker (will NOT process low-priority jobs) + +cd "${KPI_SRC_DIR}" + +AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" +AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" + +exec celery -A kobo worker --loglevel=info \ + --hostname=kpi_main_worker@%h \ + --logfile=${KPI_LOGS_DIR}/celery_kpi_worker.log \ + --pidfile=/tmp/celery_kpi_worker.pid \ + --queues=kpi_queue \ + --exclude-queues=kpi_low_priority_queue,kobocat_queue \ + --uid=${UWSGI_USER} \ + --gid=${UWSGI_GROUP} \ + --autoscale ${AUTOSCALE_MIN},${AUTOSCALE_MAX} From 7f533a576e0f31cd546bb9588df0e2ecfab1bd0c Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 5 Apr 2024 16:23:16 -0400 Subject: [PATCH 04/21] Rename celery workers --- docker/entrypoint_celery_kobocat_worker.bash | 2 +- docker/entrypoint_celery_kpi_low_priority_worker.bash | 2 +- docker/entrypoint_celery_kpi_worker.bash | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/entrypoint_celery_kobocat_worker.bash b/docker/entrypoint_celery_kobocat_worker.bash index 85ca74f2a2..68e5c77a14 100755 --- a/docker/entrypoint_celery_kobocat_worker.bash +++ b/docker/entrypoint_celery_kobocat_worker.bash @@ -10,7 +10,7 @@ AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" exec celery -A kobo worker --loglevel=info \ - --hostname=kobocat_main_worker@%h \ + --hostname=kobocat_worker@%h \ --logfile=${KPI_LOGS_DIR}/celery_kobocat_worker.log \ --pidfile=/tmp/celery_kobocat_worker.pid \ --queues=kobocat_queue \ diff --git a/docker/entrypoint_celery_kpi_low_priority_worker.bash b/docker/entrypoint_celery_kpi_low_priority_worker.bash index 5008fde4dd..de8323f627 100755 --- a/docker/entrypoint_celery_kpi_low_priority_worker.bash +++ b/docker/entrypoint_celery_kpi_low_priority_worker.bash @@ -10,7 +10,7 @@ AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" exec celery -A kobo worker --loglevel=info \ - --hostname=kpi_main_worker@%h \ + --hostname=kpi_low_priority_worker@%h \ --logfile=${KPI_LOGS_DIR}/celery_kpi_low_priority_worker.log \ --pidfile=/tmp/celery_kpi_low_priority_worker.pid \ --queues=kpi_low_priority_queue \ diff --git a/docker/entrypoint_celery_kpi_worker.bash b/docker/entrypoint_celery_kpi_worker.bash index d44d6a8d6b..ba193c2b44 100755 --- a/docker/entrypoint_celery_kpi_worker.bash +++ b/docker/entrypoint_celery_kpi_worker.bash @@ -10,7 +10,7 @@ AUTOSCALE_MIN="${CELERY_AUTOSCALE_MIN:-2}" AUTOSCALE_MAX="${CELERY_AUTOSCALE_MAX:-6}" exec celery -A kobo worker --loglevel=info \ - --hostname=kpi_main_worker@%h \ + --hostname=kpi_worker@%h \ --logfile=${KPI_LOGS_DIR}/celery_kpi_worker.log \ --pidfile=/tmp/celery_kpi_worker.pid \ --queues=kpi_queue \ From 55ad7f608bb28c91e88d5283f2a271997a075fd5 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 5 Apr 2024 18:17:37 -0400 Subject: [PATCH 05/21] Fix WSGI detection --- docker/entrypoint.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 33acad6027..c88e93a513 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -13,8 +13,8 @@ if [[ -z $DATABASE_URL ]]; then fi # Handle Python dependencies BEFORE attempting any `manage.py` commands -KPI_WEB_SERVER="${UWSGI:-uWSGI}" -if [[ "${KPI_WEB_SERVER,,}" == 'uwsgi' ]]; then +WSGI="${WSGI:-uWSGI}" +if [[ "${WSGI}" == 'uWSGI' ]]; then # `diff` returns exit code 1 if it finds a difference between the files if ! diff -q "${KPI_SRC_DIR}/dependencies/pip/requirements.txt" "${TMP_DIR}/pip_dependencies.txt" then @@ -90,7 +90,8 @@ chown -R "${UWSGI_USER}:${UWSGI_GROUP}" "${KPI_MEDIA_DIR}" echo 'KPI initialization completed.' cd "${KPI_SRC_DIR}" -if [[ "${UWSGI,,}" == 'uwsgi' ]]; then + +if [[ "${WSGI}" == 'uWSGI' ]]; then echo "Running \`kpi\` container with uWSGI application server." $(command -v uwsgi) --ini ${KPI_SRC_DIR}/docker/uwsgi.ini else From acd258c6f5787f6920ecf43bea5f45c0f589256f Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 16 Apr 2024 08:39:05 -0400 Subject: [PATCH 06/21] WIP first migration --- kobo/apps/kobo_auth/models.py | 2 +- kobo/settings/base.py | 4 ++ kpi/utils/monkey_patching.py | 4 +- .../add_migration_from_custom_user_model.py | 44 +++++++++++++++++++ scripts/migrate.sh | 6 +++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 scripts/add_migration_from_custom_user_model.py create mode 100755 scripts/migrate.sh diff --git a/kobo/apps/kobo_auth/models.py b/kobo/apps/kobo_auth/models.py index e63d20bbb9..fad4e603e5 100644 --- a/kobo/apps/kobo_auth/models.py +++ b/kobo/apps/kobo_auth/models.py @@ -34,7 +34,7 @@ def has_perm(self, perm, obj=None): return super().has_perm(perm, obj) if perm in get_model_permission_codenames(): - with use_db(settings.OPENROSA_DB_ALIAS): + with use_db(settings.OPENROA_DB_ALIAS): return super().has_perm(perm, obj) # Otherwise, check in KPI DB diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 0bcd600c6d..05d5138f45 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -1732,3 +1732,7 @@ def dj_stripe_request_callback_method(): 'application/zip', 'application/x-zip-compressed' ] + +# Silence Django Guardian warning. Authentication backend is hooked, but +# Django Guardian does not recognize it because it is extended +SILENCED_SYSTEM_CHECKS = ['guardian.W001'] diff --git a/kpi/utils/monkey_patching.py b/kpi/utils/monkey_patching.py index a2f832d614..6b0251e572 100644 --- a/kpi/utils/monkey_patching.py +++ b/kpi/utils/monkey_patching.py @@ -10,8 +10,8 @@ def create_permissions(app_config, using=DEFAULT_DB_ALIAS, **kwargs): """ - Avoid to create permissions on the wrong database when post signal is emitted - on migrations + Avoid to create permissions on the wrong database when post signal is + emitted on migrations """ if ( app_config.label in OPENROSA_APP_LABELS diff --git a/scripts/add_migration_from_custom_user_model.py b/scripts/add_migration_from_custom_user_model.py new file mode 100644 index 0000000000..13a8fd1474 --- /dev/null +++ b/scripts/add_migration_from_custom_user_model.py @@ -0,0 +1,44 @@ +from django.conf import settings +from django.db import connection, connections + + +def run(): + """ + Insert an entry in django_migrations for existing setups. + Because kobo_auth.User becomes the (custom) User model, we enter + a chicken or the egg condition, i.e.: some migrations depend on the User + model to be created first, but custom User is not and those migrations are + already applied. It makes Django raise an InconsistentMigrationHistory error. + """ + with connection.cursor() as cursor: + cursor.execute( + "SELECT id FROM django_migrations " + "WHERE app = 'auth' AND name = '0012_alter_user_first_name_max_length';" + ) + row = cursor.fetchone() + + if row: + cursor.execute( + "SELECT id FROM django_migrations " + "WHERE app = 'kobo_auth' AND name = '0001_initial';" + ) + row = cursor.fetchone() + if not row: + cursor.execute( + "INSERT INTO django_migrations (app, name, applied) " + "VALUES ('kobo_auth', '0001_initial', NOW());" + "UPDATE django_content_type SET app_label = 'kobo_auth' " + "WHERE app_label = 'auth' and model = 'user';" + ) + + with connections[settings.OPENROSA_DB_ALIAS].cursor() as kc_cursor: + kc_cursor.execute( + "INSERT INTO django_migrations (app, name, applied) " + "VALUES ('kobo_auth', '0001_initial', NOW());" + "UPDATE django_content_type SET app_label = 'kobo_auth' " + "WHERE app_label = 'auth' and model = 'user';" + ) + else: + print('Migration kobo_auth.0001 already applied. Skip!') + else: + raise Exception('Run `./manage.py migrate auth` first') diff --git a/scripts/migrate.sh b/scripts/migrate.sh new file mode 100755 index 0000000000..1c68595738 --- /dev/null +++ b/scripts/migrate.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +set -e + +python manage.py runscript add_migration_from_custom_user_model +python manage.py migrate --noinput +python manage.py migrate --noinput --database kobocat From 3fd16f6f5e4bb1ac8d72a298ebd4f6553fb6fcae Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 18 Apr 2024 09:53:35 -0400 Subject: [PATCH 07/21] Change main.0014 migration to handle kobocat connection --- .../0014_drop_old_formdisclaimer_tables.py | 4 ++-- scripts/add_migration_from_custom_user_model.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/kobo/apps/openrosa/apps/main/migrations/0014_drop_old_formdisclaimer_tables.py b/kobo/apps/openrosa/apps/main/migrations/0014_drop_old_formdisclaimer_tables.py index f8f01ea12a..eb5252e087 100644 --- a/kobo/apps/openrosa/apps/main/migrations/0014_drop_old_formdisclaimer_tables.py +++ b/kobo/apps/openrosa/apps/main/migrations/0014_drop_old_formdisclaimer_tables.py @@ -1,6 +1,6 @@ # Generated by Django 3.2.15 on 2023-03-22 14:05 -from django.db import migrations, connection +from django.db import migrations, connections from django.conf import settings @@ -28,7 +28,7 @@ def get_operations(): WHERE nsp.nspname = 'public' AND rel.relname = %s; """ - with connection.cursor() as cursor: + with connections[settings.OPENROSA_DB_ALIAS].cursor() as cursor: drop_table_queries = [] for table in tables: cursor.execute(sql, [table]) diff --git a/scripts/add_migration_from_custom_user_model.py b/scripts/add_migration_from_custom_user_model.py index 13a8fd1474..12015e240c 100644 --- a/scripts/add_migration_from_custom_user_model.py +++ b/scripts/add_migration_from_custom_user_model.py @@ -3,6 +3,23 @@ def run(): + migrate_custom_user_model() + delete_kobocat_form_disclaimer_app() + + +def delete_kobocat_form_disclaimer_app(): + """ + Kobocat form_disclaimer app does not exist anymore but its migrations + create conflicts and must be deleted before applying migration. + """ + with connections[settings.OPENROSA_DB_ALIAS].cursor() as kc_cursor: + kc_cursor.execute( + "DELETE FROM django_migrations " + "WHERE app_label = 'form_disclaimer';" + ) + + +def migrate_custom_user_model(): """ Insert an entry in django_migrations for existing setups. Because kobo_auth.User becomes the (custom) User model, we enter From 47a9be5aa20c2ab2c6347599d74f04b9ea196b05 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 23 May 2024 10:07:41 -0400 Subject: [PATCH 08/21] Fix fix-migrations script --- docker/entrypoint.sh | 2 +- ...user_model.py => fix_migrations_for_kobocat_django_app.py} | 4 ++-- scripts/migrate.sh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename scripts/{add_migration_from_custom_user_model.py => fix_migrations_for_kobocat_django_app.py} (95%) diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index c88e93a513..6a69a2383a 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -36,7 +36,7 @@ fi /bin/bash "${INIT_PATH}/wait_for_postgres.bash" echo 'Running migrations…' -gosu "${UWSGI_USER}" python manage.py migrate --noinput +gosu "${UWSGI_USER}" scripts/migrate.sh echo 'Creating superuser…' gosu "${UWSGI_USER}" python manage.py create_kobo_superuser diff --git a/scripts/add_migration_from_custom_user_model.py b/scripts/fix_migrations_for_kobocat_django_app.py similarity index 95% rename from scripts/add_migration_from_custom_user_model.py rename to scripts/fix_migrations_for_kobocat_django_app.py index 12015e240c..40c847d653 100644 --- a/scripts/add_migration_from_custom_user_model.py +++ b/scripts/fix_migrations_for_kobocat_django_app.py @@ -9,13 +9,13 @@ def run(): def delete_kobocat_form_disclaimer_app(): """ - Kobocat form_disclaimer app does not exist anymore but its migrations + KoboCAT form_disclaimer app does not exist anymore but its migrations create conflicts and must be deleted before applying migration. """ with connections[settings.OPENROSA_DB_ALIAS].cursor() as kc_cursor: kc_cursor.execute( "DELETE FROM django_migrations " - "WHERE app_label = 'form_disclaimer';" + "WHERE app = 'form_disclaimer';" ) diff --git a/scripts/migrate.sh b/scripts/migrate.sh index 1c68595738..79d6c679ff 100755 --- a/scripts/migrate.sh +++ b/scripts/migrate.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash set -e -python manage.py runscript add_migration_from_custom_user_model +python manage.py runscript fix_migrations_for_kobocat_django_app python manage.py migrate --noinput python manage.py migrate --noinput --database kobocat From d9ff55657760540c5c7d5cf636770fb1a1b0e1a3 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 24 May 2024 10:49:08 -0400 Subject: [PATCH 09/21] Fix typo --- kobo/apps/kobo_auth/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/kobo_auth/models.py b/kobo/apps/kobo_auth/models.py index fad4e603e5..e63d20bbb9 100644 --- a/kobo/apps/kobo_auth/models.py +++ b/kobo/apps/kobo_auth/models.py @@ -34,7 +34,7 @@ def has_perm(self, perm, obj=None): return super().has_perm(perm, obj) if perm in get_model_permission_codenames(): - with use_db(settings.OPENROA_DB_ALIAS): + with use_db(settings.OPENROSA_DB_ALIAS): return super().has_perm(perm, obj) # Otherwise, check in KPI DB From 7b0e1c206fbf3d25e062c41b4dca5733a16f76cc Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 28 May 2024 11:38:29 -0400 Subject: [PATCH 10/21] Make attachment resizing work again --- .../commands/create_image_thumbnails.py | 15 ++---- .../openrosa/apps/logger/models/attachment.py | 12 ++--- kobo/apps/openrosa/apps/logger/signals.py | 6 +++ kobo/apps/openrosa/libs/utils/image_tools.py | 36 +++++++++----- kobo/apps/openrosa/libs/utils/viewer_tools.py | 10 ++-- kobo/settings/base.py | 14 ++---- kpi/deployment_backends/base_backend.py | 49 ++++++++++++++----- .../kc_access/shadow_models.py | 19 ++++++- kpi/deployment_backends/kobocat_backend.py | 6 +-- kpi/tests/utils/mock.py | 8 +-- kpi/views/v2/attachment.py | 26 +++++++++- 11 files changed, 133 insertions(+), 68 deletions(-) diff --git a/kobo/apps/openrosa/apps/logger/management/commands/create_image_thumbnails.py b/kobo/apps/openrosa/apps/logger/management/commands/create_image_thumbnails.py index b5da962417..c07c625294 100644 --- a/kobo/apps/openrosa/apps/logger/management/commands/create_image_thumbnails.py +++ b/kobo/apps/openrosa/apps/logger/management/commands/create_image_thumbnails.py @@ -10,7 +10,7 @@ from kobo.apps.openrosa.apps.logger.models.xform import XForm from kobo.apps.openrosa.libs.utils.image_tools import resize from kobo.apps.openrosa.libs.utils.model_tools import queryset_iterator -from kobo.apps.openrosa.libs.utils.viewer_tools import get_path +from kobo.apps.openrosa.libs.utils.viewer_tools import get_optimized_image_path from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) @@ -60,12 +60,10 @@ def handle(self, *args, **kwargs): for att in queryset_iterator(attachments_qs): filename = att.media_file.name - full_path = get_path( - filename, settings.THUMB_CONF['small']['suffix'] - ) + full_path = get_optimized_image_path(filename, 'small') if kwargs.get('force') is not None: - for s in settings.THUMB_CONF.keys(): - fp = get_path(filename, settings.THUMB_CONF[s]['suffix']) + for suffix in settings.THUMB_CONF.keys(): + fp = get_optimized_image_path(filename, suffix) if default_storage.exists(fp): default_storage.delete(fp) @@ -73,10 +71,7 @@ def handle(self, *args, **kwargs): try: resize(filename) if default_storage.exists( - get_path( - filename, - '%s' % settings.THUMB_CONF['small']['suffix'], - ) + get_optimized_image_path(filename, 'small') ): print( 'Thumbnails created for %(file)s' diff --git a/kobo/apps/openrosa/apps/logger/models/attachment.py b/kobo/apps/openrosa/apps/logger/models/attachment.py index 9a0ade9fea..024ee06b11 100644 --- a/kobo/apps/openrosa/apps/logger/models/attachment.py +++ b/kobo/apps/openrosa/apps/logger/models/attachment.py @@ -90,18 +90,18 @@ def file_hash(self): def filename(self): return os.path.basename(self.media_file.name) - def secure_url(self, suffix="original"): + def secure_url(self, suffix: str = 'original'): """ - Returns image URL through kobocat redirector. + Returns image URL through KoboCAT redirector. :param suffix: str. original|large|medium|small :return: str """ - if suffix != "original" and suffix not in settings.THUMB_CONF.keys(): - raise Exception("Invalid image thumbnail") + if suffix != 'original' and suffix not in settings.THUMB_CONF.keys(): + raise Exception('Invalid image thumbnail') - return "{kobocat_url}{media_url}{suffix}?{media_file}".format( + return '{kobocat_url}{media_url}{suffix}?{media_file}'.format( kobocat_url=settings.KOBOCAT_URL, media_url=settings.MEDIA_URL, suffix=suffix, - media_file=urlencode({"media_file": self.media_file.name}) + media_file=urlencode({'media_file': self.media_file.name}) ) diff --git a/kobo/apps/openrosa/apps/logger/signals.py b/kobo/apps/openrosa/apps/logger/signals.py index 5f9fd50929..ec2013e57a 100644 --- a/kobo/apps/openrosa/apps/logger/signals.py +++ b/kobo/apps/openrosa/apps/logger/signals.py @@ -1,6 +1,7 @@ # coding: utf-8 import logging +from django.conf import settings from django.db import transaction from django.db.models import F from django.db.models.signals import ( @@ -12,6 +13,7 @@ from kobo.apps.openrosa.apps.logger.models.attachment import Attachment from kobo.apps.openrosa.apps.logger.models.xform import XForm from kobo.apps.openrosa.apps.main.models.user_profile import UserProfile +from kobo.apps.openrosa.libs.utils.image_tools import get_optimized_image_path from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) @@ -60,6 +62,10 @@ def pre_delete_attachment(instance, **kwargs): # `attachment.save()` behind the scene which would call again the `post_save` # signal below. Bonus: it avoids an extra query 😎. default_storage.delete(media_file_name) + for suffix in settings.THUMB_CONF: + default_storage.delete( + get_optimized_image_path(media_file_name, suffix) + ) except Exception as e: logging.error('Failed to delete attachment: ' + str(e), exc_info=True) diff --git a/kobo/apps/openrosa/libs/utils/image_tools.py b/kobo/apps/openrosa/libs/utils/image_tools.py index d2571eddda..61e8684e60 100644 --- a/kobo/apps/openrosa/libs/utils/image_tools.py +++ b/kobo/apps/openrosa/libs/utils/image_tools.py @@ -1,4 +1,5 @@ # coding: utf-8 +import os from io import BytesIO from tempfile import NamedTemporaryFile @@ -8,7 +9,7 @@ from django.core.files.storage import FileSystemStorage from PIL import Image -from kobo.apps.openrosa.libs.utils.viewer_tools import get_path +from kobo.apps.openrosa.libs.utils.viewer_tools import get_optimized_image_path from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) @@ -55,15 +56,16 @@ def _save_thumbnails(image, original_path, size, suffix): image.convert('RGB').save(nm.name) # Try to delete file with the same name if it already exists to avoid useless file. - # i.e if `file_.jpg` exists, Storage will save `a__.jpg` + # i.e. if `file_.jpg` exists, Storage will save `a__.jpg` # but nothing in the code is aware about this ` try: - default_storage.delete(get_path(original_path, suffix)) + default_storage.delete(get_optimized_image_path(original_path, suffix)) except IOError: pass default_storage.save( - get_path(original_path, suffix), ContentFile(nm.read())) + get_optimized_image_path(original_path, suffix), ContentFile(nm.read()) + ) nm.close() @@ -84,11 +86,12 @@ def resize(filename): image = Image.open(im) if image: - conf = settings.THUMB_CONF - [_save_thumbnails( - image, original_path, - conf[key]['size'], - conf[key]['suffix']) for key in settings.THUMB_ORDER] + [ + _save_thumbnails( + image, original_path, size, suffix + ) + for suffix, size in settings.THUMB_CONF.items() + ] def image_url(attachment, suffix): @@ -101,13 +104,20 @@ def image_url(attachment, suffix): return url else: if suffix in settings.THUMB_CONF: - size = settings.THUMB_CONF[suffix]['suffix'] filename = attachment.media_file.name if default_storage.exists(filename): - if default_storage.exists(get_path(filename, size)) and \ - default_storage.size(get_path(filename, size)) > 0: + if ( + default_storage.exists( + get_optimized_image_path(filename, suffix) + ) + and default_storage.size( + get_optimized_image_path(filename, suffix) + ) + > 0 + ): url = default_storage.url( - get_path(filename, size)) + get_optimized_image_path(filename, suffix) + ) else: resize(filename) return image_url(attachment, suffix) diff --git a/kobo/apps/openrosa/libs/utils/viewer_tools.py b/kobo/apps/openrosa/libs/utils/viewer_tools.py index 16cda45572..5b6b83fb3a 100644 --- a/kobo/apps/openrosa/libs/utils/viewer_tools.py +++ b/kobo/apps/openrosa/libs/utils/viewer_tools.py @@ -35,9 +35,9 @@ def format_date_for_mongo(x): ) -def get_path(path, suffix): - fileName, fileExtension = os.path.splitext(path) - return fileName + suffix + fileExtension +def get_optimized_image_path(path: str, suffix: str) -> str: + file_name, ext = os.path.splitext(path) + return f'{file_name}-{suffix}{ext}' def image_urls_dict(instance): @@ -51,10 +51,8 @@ def image_urls_dict(instance): :return: dict """ urls = dict() - # Remove leading dash from suffix - suffix = settings.THUMB_CONF['medium']['suffix'][1:] for a in instance.attachments.all(): - urls[a.filename] = a.secure_url(suffix=suffix) + urls[a.filename] = a.secure_url(suffix='medium') return urls diff --git a/kobo/settings/base.py b/kobo/settings/base.py index d0a472fffe..471bf8396f 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -1583,12 +1583,6 @@ def dj_stripe_request_callback_method(): add_type('application/geo+json', '.geojson') KOBOCAT_MEDIA_URL = f'{KOBOCAT_URL}/media/' -KOBOCAT_THUMBNAILS_SUFFIX_MAPPING = { - 'original': '', - 'large': '_large', - 'medium': '_medium', - 'small': '_small', -} TRENCH_AUTH = { 'USER_MFA_MODEL': 'mfa.MfaMethod', @@ -1734,12 +1728,10 @@ def dj_stripe_request_callback_method(): ] THUMB_CONF = { - 'large': {'size': 1280, 'suffix': '-large'}, - 'medium': {'size': 640, 'suffix': '-medium'}, - 'small': {'size': 240, 'suffix': '-small'}, + 'large': 1280, + 'medium': 640, + 'small': 240, } -# order of thumbnails from largest to smallest -THUMB_ORDER = ['large', 'medium', 'small'] SUPPORTED_MEDIA_UPLOAD_TYPES = [ 'image/jpeg', diff --git a/kpi/deployment_backends/base_backend.py b/kpi/deployment_backends/base_backend.py index 6787619b36..8de2a4faf4 100644 --- a/kpi/deployment_backends/base_backend.py +++ b/kpi/deployment_backends/base_backend.py @@ -42,6 +42,7 @@ xml_tostring, ) + class BaseDeploymentBackend(abc.ABC): """ Defines the interface for a deployment backend. @@ -775,20 +776,42 @@ def _rewrite_json_attachment_urls( ) for attachment in submission['_attachments']: - for size, suffix in settings.KOBOCAT_THUMBNAILS_SUFFIX_MAPPING.items(): - # We should use 'attachment-list' with `?xpath=` but we do not - # know what the XPath is here. Since the primary key is already - # exposed, let's use it to build the url with 'attachment-detail' - kpi_url = reverse( - 'attachment-detail', - args=(self.asset.uid, submission['_id'], attachment['id']), - request=request, - ) - key = f'download{suffix}_url' - try: + # We should use 'attachment-list' with `?xpath=` but we do not + # know what the XPath is here. Since the primary key is already + # exposed, let's use it to build the url. + kpi_url = reverse( + 'attachment-detail', + args=( + self.asset.uid, + submission['_id'], + attachment['id'], + ), + request=request, + ) + key = f'download_url' + attachment[key] = kpi_url + if attachment['mimetype'].startswith('image/'): + for suffix in settings.THUMB_CONF.keys(): + kpi_url = reverse( + 'attachment-thumb', + args=( + self.asset.uid, + submission['_id'], + attachment['id'], + suffix, + ), + request=request, + ) + key = f'download_{suffix}_url' attachment[key] = kpi_url - except KeyError: - continue + else: + for suffix in settings.THUMB_CONF.keys(): + try: + key = f'download_{suffix}_url' + del attachment[key] + except KeyError: + continue + filename = attachment['filename'] attachment['filename'] = os.path.join( self.asset.owner.username, diff --git a/kpi/deployment_backends/kc_access/shadow_models.py b/kpi/deployment_backends/kc_access/shadow_models.py index d049437ad6..d0ceede889 100644 --- a/kpi/deployment_backends/kc_access/shadow_models.py +++ b/kpi/deployment_backends/kc_access/shadow_models.py @@ -19,7 +19,14 @@ from django.utils import timezone from django_digest.models import PartialDigest +from kobo.apps.openrosa.libs.utils.image_tools import ( + get_optimized_image_path, + resize, +) from kpi.constants import SHADOW_MODEL_APP_LABEL +from kpi.deployment_backends.kc_access.storage import ( + default_kobocat_storage, +) from kpi.exceptions import ( BadContentTypeException, ) @@ -163,7 +170,9 @@ def mp3_storage_path(self): """ return f'{self.storage_path}.mp3' - def protected_path(self, format_: Optional[str] = None): + def protected_path( + self, format_: Optional[str] = None, size: Optional[str] = None + ) -> str: """ Return path to be served as protected file served by NGINX """ @@ -173,6 +182,14 @@ def protected_path(self, format_: Optional[str] = None): else: attachment_file_path = self.absolute_path + if size and self.mimetype.startswith('image/'): + optimized_image_path = get_optimized_image_path( + self.media_file.name, size + ) + if not default_kobocat_storage.exists(optimized_image_path): + resize(self.media_file.name) + attachment_file_path = default_kobocat_storage.path(optimized_image_path) + if isinstance(get_kobocat_storage(), KobocatFileSystemStorage): # Django normally sanitizes accented characters in file names during # save on disk but some languages have extra letters diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index eea5e026ca..227b15c194 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -792,9 +792,9 @@ def get_submissions( """ mongo_query_params['submission_ids'] = submission_ids - params = self.validate_submission_list_params(user, - format_type=format_type, - **mongo_query_params) + params = self.validate_submission_list_params( + user, format_type=format_type, **mongo_query_params + ) if format_type == SUBMISSION_FORMAT_TYPE_JSON: submissions = self.__get_submissions_in_json(request, **params) diff --git a/kpi/tests/utils/mock.py b/kpi/tests/utils/mock.py index e9d350d2ed..240b2c6140 100644 --- a/kpi/tests/utils/mock.py +++ b/kpi/tests/utils/mock.py @@ -134,11 +134,13 @@ def __exit__(self, exc_type, exc_val, exc_tb): def absolute_path(self): return self.media_file.path - def protected_path(self, format_: Optional[str] = None): + def protected_path( + self, format_: Optional[str] = None, size: Optional[str] = None + ) -> str: if format_ == 'mp3': - suffix = f'.mp3' + suffix = '.mp3' with NamedTemporaryFile(suffix=suffix) as f: - self.content = self.get_transcoded_audio('mp3') + self.content = self.get_transcoded_audio(format_) return f.name else: return self.absolute_path diff --git a/kpi/views/v2/attachment.py b/kpi/views/v2/attachment.py index 6108c00759..2f552a2399 100644 --- a/kpi/views/v2/attachment.py +++ b/kpi/views/v2/attachment.py @@ -5,6 +5,7 @@ from django.shortcuts import Http404 from django.utils.translation import gettext as t from rest_framework import viewsets, serializers +from rest_framework.decorators import action from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin @@ -20,6 +21,10 @@ from kpi.renderers import MediaFileRenderer, MP3ConversionRenderer from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin +thumbnail_suffixes_pattern = 'original|' + '|'.join( + [suffix for suffix in settings.THUMB_CONF] +) + class AttachmentViewSet( NestedViewSetMixin, @@ -62,7 +67,22 @@ def retrieve(self, request, pk, *args, **kwargs): # Since endpoint is needed for KobocatDeploymentBackend to overwrite # Mongo attachments URL with their primary keys (instead of their XPath) submission_id_or_uuid = kwargs['parent_lookup_data'] - return self._get_response(request, submission_id_or_uuid, attachment_id=pk) + return self._get_response( + request, + submission_id_or_uuid, + attachment_id=pk, + size=kwargs.get('size'), + ) + + @action( + detail=True, + methods=['GET'], + url_path=f'(?P({thumbnail_suffixes_pattern}))' + ) + def thumb(self, request, pk, size, *args, **kwargs): + if size != 'original': + kwargs['size'] = size + return self.retrieve(request, pk, *args, **kwargs) def list(self, request, *args, **kwargs): submission_id_or_uuid = kwargs['parent_lookup_data'] @@ -81,6 +101,7 @@ def _get_response( submission_id_or_uuid: Union[str, int], attachment_id: Optional[int] = None, xpath: Optional[str] = None, + size: Optional[str] = None, ) -> Response: try: @@ -100,7 +121,8 @@ def _get_response( try: protected_path = attachment.protected_path( - request.accepted_renderer.format + format_=request.accepted_renderer.format, + size=size, ) except FFMpegException: raise serializers.ValidationError({ From 70cda9e4b9f479aa359160c791266e491119a492 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 28 May 2024 16:29:37 -0400 Subject: [PATCH 11/21] Add tests for resize --- .../logger/tests/models/test_attachment.py | 40 ++++--- .../apps/viewer/models/data_dictionary.py | 3 +- .../tests/test_submission_extras_api_post.py | 4 +- .../kc_access/shadow_models.py | 7 +- kpi/tests/api/v2/test_api_attachments.py | 110 ++++++++++++++++++ kpi/tests/utils/mock.py | 59 +++++++--- kpi/views/v2/attachment.py | 14 +-- 7 files changed, 192 insertions(+), 45 deletions(-) diff --git a/kobo/apps/openrosa/apps/logger/tests/models/test_attachment.py b/kobo/apps/openrosa/apps/logger/tests/models/test_attachment.py index b9d770b620..693e49e4e8 100644 --- a/kobo/apps/openrosa/apps/logger/tests/models/test_attachment.py +++ b/kobo/apps/openrosa/apps/logger/tests/models/test_attachment.py @@ -12,7 +12,6 @@ default_kobocat_storage as default_storage, ) - class TestAttachment(TestBase): def setUp(self): @@ -20,12 +19,18 @@ def setUp(self): self._publish_transportation_form_and_submit_instance() self.media_file = "1335783522563.jpg" media_file = os.path.join( - self.this_directory, 'fixtures', - 'transportation', 'instances', self.surveys[0], self.media_file) + self.this_directory, + 'fixtures', + 'transportation', + 'instances', + self.surveys[0], + self.media_file, + ) self.instance = Instance.objects.all()[0] self.attachment = Attachment.objects.create( instance=self.instance, - media_file=File(open(media_file, 'rb'), media_file)) + media_file=File(open(media_file, 'rb'), media_file), + ) def test_mimetype(self): self.assertEqual(self.attachment.mimetype, 'image/jpeg') @@ -35,13 +40,16 @@ def test_thumbnails(self): url = image_url(attachment, 'small') filename = attachment.media_file.name.replace('.jpg', '') thumbnail = '%s-small.jpg' % filename - self.assertNotEqual( - url.find(thumbnail), -1) + self.assertNotEqual(url.find(thumbnail), -1) for size in ['small', 'medium', 'large']: thumbnail = f'{filename}-{size}.jpg' - self.assertTrue( - default_storage.exists(thumbnail)) - default_storage.delete(thumbnail) + self.assertTrue(default_storage.exists(thumbnail)) + + # Ensure clean-up is ok + attachment.delete() + for size in ['small', 'medium', 'large']: + thumbnail = f'{filename}-{size}.jpg' + self.assertFalse(default_storage.exists(thumbnail)) def test_create_thumbnails_command(self): call_command("create_image_thumbnails") @@ -50,17 +58,19 @@ def test_create_thumbnails_command(self): filename = attachment.media_file.name.replace('.jpg', '') for size in settings.THUMB_CONF.keys(): thumbnail = '%s-%s.jpg' % (filename, size) - self.assertTrue( - default_storage.exists(thumbnail)) - created_times[size] = default_storage.get_modified_time(thumbnail) + self.assertTrue(default_storage.exists(thumbnail)) + created_times[size] = default_storage.get_modified_time( + thumbnail + ) # replace or regenerate thumbnails if they exist call_command("create_image_thumbnails", force=True) for attachment in Attachment.objects.filter(instance=self.instance): filename = attachment.media_file.name.replace('.jpg', '') for size in settings.THUMB_CONF.keys(): thumbnail = f'{filename}-{size}.jpg' + self.assertTrue(default_storage.exists(thumbnail)) self.assertTrue( - default_storage.exists(thumbnail)) - self.assertTrue( - default_storage.get_modified_time(thumbnail) > created_times[size]) + default_storage.get_modified_time(thumbnail) + > created_times[size] + ) default_storage.delete(thumbnail) diff --git a/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py b/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py index 01149538f2..f2f02d9f16 100644 --- a/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py +++ b/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py @@ -177,8 +177,7 @@ def get_survey(self): self._survey = \ builder.create_survey_element_from_json(self.json) except ValueError: - xml = bytes(bytearray(self.xml, encoding='utf-8')) - self._survey = create_survey_element_from_xml(xml) + self._survey = create_survey_element_from_xml(self.xml) return self._survey survey = property(get_survey) diff --git a/kobo/apps/subsequences/tests/test_submission_extras_api_post.py b/kobo/apps/subsequences/tests/test_submission_extras_api_post.py index fb415e5432..f530d870ee 100644 --- a/kobo/apps/subsequences/tests/test_submission_extras_api_post.py +++ b/kobo/apps/subsequences/tests/test_submission_extras_api_post.py @@ -429,10 +429,10 @@ def test_google_transcript_post(self, m1, m2): 'submission': submission_id, 'q1': {GOOGLETS: {'status': 'requested', 'languageCode': ''}} } - with self.assertNumQueries(FuzzyInt(51, 55)): + with self.assertNumQueries(FuzzyInt(51, 57)): res = self.client.post(url, data, format='json') self.assertContains(res, 'complete') - with self.assertNumQueries(FuzzyInt(20, 24)): + with self.assertNumQueries(FuzzyInt(20, 26)): self.client.post(url, data, format='json') @override_settings(CACHES={'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache'}}) diff --git a/kpi/deployment_backends/kc_access/shadow_models.py b/kpi/deployment_backends/kc_access/shadow_models.py index d0ceede889..155804d03d 100644 --- a/kpi/deployment_backends/kc_access/shadow_models.py +++ b/kpi/deployment_backends/kc_access/shadow_models.py @@ -171,20 +171,19 @@ def mp3_storage_path(self): return f'{self.storage_path}.mp3' def protected_path( - self, format_: Optional[str] = None, size: Optional[str] = None + self, format_: Optional[str] = None, suffix: Optional[str] = None ) -> str: """ Return path to be served as protected file served by NGINX """ - if format_ == 'mp3': attachment_file_path = self.absolute_mp3_path else: attachment_file_path = self.absolute_path - if size and self.mimetype.startswith('image/'): + if suffix and self.mimetype.startswith('image/'): optimized_image_path = get_optimized_image_path( - self.media_file.name, size + self.media_file.name, suffix ) if not default_kobocat_storage.exists(optimized_image_path): resize(self.media_file.name) diff --git a/kpi/tests/api/v2/test_api_attachments.py b/kpi/tests/api/v2/test_api_attachments.py index 92fbd40fc7..3ceb4ae8e9 100644 --- a/kpi/tests/api/v2/test_api_attachments.py +++ b/kpi/tests/api/v2/test_api_attachments.py @@ -1,10 +1,19 @@ import uuid +from django.conf import settings +from django.core.files.base import File from django.http import QueryDict from django.urls import reverse from rest_framework import status from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.openrosa.apps.main.models import UserProfile +from kobo.apps.openrosa.apps.logger.models import XForm, Instance, Attachment +from kobo.apps.openrosa.apps.logger.models.attachment import upload_to +from kobo.apps.openrosa.apps.viewer.models import ParsedInstance +from kpi.deployment_backends.kc_access.storage import ( + default_kobocat_storage as default_storage, +) from kpi.models import Asset from kpi.tests.base_test_case import BaseAssetTestCase from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE @@ -261,3 +270,104 @@ def test_get_attachment_with_submission_uuid(self): response = self.client.get(url) assert response.status_code == status.HTTP_200_OK assert response['Content-Type'] == 'video/3gpp' + + def test_thumbnail_creation_on_demand(self): + media_file = settings.BASE_DIR + '/kpi/tests/audio_conversion_test_image.jpg' + + xform_xml = f""" + + + Project with attachments + + + <{self.asset.uid} id="{self.asset.uid}"> + + + + + <__version__/> + + + + + + + + + + + + + + + + + + """ + + instance_xml = f""" + <{self.asset.uid} xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" id="{self.asset.uid}"> + + 027e8acb31b24acebb7f6b2a74ac1ff3 + + audio_conversion_test_image.jpg + <__version__>vd3dpf3fL2C8abWG4EPJWC + + uuid:ba82fbca-9a05-45c7-afb6-295c90f838e5 + + + """ + + UserProfile.objects.get_or_create(user=self.someuser) + xform = XForm.objects.create( + user=self.someuser, + xml=xform_xml, + id_string=self.asset.uid, + kpi_asset_uid=self.asset.uid + ) + instance = Instance.objects.create(xform=xform, xml=instance_xml) + attachment = Attachment.objects.create(instance=instance) + attachment.media_file = File( + open(media_file, 'rb'), upload_to(attachment, media_file) + ) + attachment.save() + + pi = ParsedInstance.objects.create(instance=instance) + self.asset.deployment.mock_submissions( + [pi.to_dict_for_mongo()] + ) + detail_url = reverse( + self._get_endpoint('attachment-detail'), + args=( + self.asset.uid, + instance.pk, + attachment.pk + ), + ) + self.client.get(detail_url) + filename = attachment.media_file.name.replace('.jpg', '') + thumbnail = f'{filename}-small.jpg' + # Thumbs should not exist yet + self.assertFalse(default_storage.exists(thumbnail)) + + thumb_url = reverse( + self._get_endpoint('attachment-thumb'), + args=( + self.asset.uid, + instance.pk, + attachment.pk, + 'small' + ), + ) + self.client.get(thumb_url) + # Thumbs should exist + self.assertTrue(default_storage.exists(thumbnail)) + + # Clean-up + attachment.delete() diff --git a/kpi/tests/utils/mock.py b/kpi/tests/utils/mock.py index 240b2c6140..5ae646f4fb 100644 --- a/kpi/tests/utils/mock.py +++ b/kpi/tests/utils/mock.py @@ -9,8 +9,13 @@ from django.conf import settings from django.core.files import File +from django.core.files.storage import default_storage from rest_framework import status +from kobo.apps.openrosa.libs.utils.image_tools import ( + get_optimized_image_path, + resize, +) from kpi.mixins.audio_transcoding import AudioTranscodingMixin from kpi.models.asset_snapshot import AssetSnapshot from kpi.tests.utils.xml import get_form_and_submission_tag_names @@ -104,24 +109,40 @@ class MockAttachment(AudioTranscodingMixin): """ Mock object to simulate KobocatAttachment. Relationship with ReadOnlyKobocatInstance is ignored but could be implemented + + TODO Remove this class and use `Attachment` model everywhere in tests """ def __init__(self, pk: int, filename: str, mimetype: str = None, **kwargs): self.id = pk # To mimic Django model instances self.pk = pk - basename = os.path.basename(filename) - file_ = os.path.join( - settings.BASE_DIR, - 'kpi', - 'tests', - basename - ) - self.media_file = File(open(file_, 'rb'), basename) - self.media_file.path = file_ - self.media_file_size = os.path.getsize(file_) + # Unit test `test_thumbnail_creation_on_demand()` is using real `Attachment` + # objects while other tests are using `MockAttachment` objects. + # If an Attachment object exists, let's assume unit test is using real + # Attachment objects. Otherwise, use MockAttachment. + from kobo.apps.openrosa.apps.logger.models import Attachment # Avoid circular import + + attachment_object = Attachment.objects.filter(pk=pk).first() + if attachment_object: + self.media_file = attachment_object.media_file + self.media_file_size = attachment_object.media_file_size + self.media_file_basename = attachment_object.media_file_basename + else: + basename = os.path.basename(filename) + file_ = os.path.join( + settings.BASE_DIR, + 'kpi', + 'tests', + basename + ) + self.media_file = File(open(file_, 'rb'), basename) + self.media_file.path = file_ + self.media_file_size = os.path.getsize(file_) + self.media_file_basename = basename + self.content = self.media_file.read() - self.media_file_basename = basename + if not mimetype: self.mimetype, _ = guess_type(file_) else: @@ -135,12 +156,20 @@ def absolute_path(self): return self.media_file.path def protected_path( - self, format_: Optional[str] = None, size: Optional[str] = None + self, format_: Optional[str] = None, suffix: Optional[str] = None ) -> str: if format_ == 'mp3': - suffix = '.mp3' - with NamedTemporaryFile(suffix=suffix) as f: + extension = '.mp3' + with NamedTemporaryFile(suffix=extension) as f: self.content = self.get_transcoded_audio(format_) return f.name else: - return self.absolute_path + if suffix and self.mimetype.startswith('image/'): + optimized_image_path = get_optimized_image_path( + self.media_file.name, suffix + ) + if not default_storage.exists(optimized_image_path): + resize(self.media_file.name) + return default_storage.path(optimized_image_path) + else: + return self.absolute_path diff --git a/kpi/views/v2/attachment.py b/kpi/views/v2/attachment.py index 2f552a2399..75aa81887f 100644 --- a/kpi/views/v2/attachment.py +++ b/kpi/views/v2/attachment.py @@ -71,17 +71,17 @@ def retrieve(self, request, pk, *args, **kwargs): request, submission_id_or_uuid, attachment_id=pk, - size=kwargs.get('size'), + suffix=kwargs.get('suffix'), ) @action( detail=True, methods=['GET'], - url_path=f'(?P({thumbnail_suffixes_pattern}))' + url_path=f'(?P({thumbnail_suffixes_pattern}))' ) - def thumb(self, request, pk, size, *args, **kwargs): - if size != 'original': - kwargs['size'] = size + def thumb(self, request, pk, suffix, *args, **kwargs): + if suffix != 'original': + kwargs['suffix'] = suffix return self.retrieve(request, pk, *args, **kwargs) def list(self, request, *args, **kwargs): @@ -101,7 +101,7 @@ def _get_response( submission_id_or_uuid: Union[str, int], attachment_id: Optional[int] = None, xpath: Optional[str] = None, - size: Optional[str] = None, + suffix: Optional[str] = None, ) -> Response: try: @@ -122,7 +122,7 @@ def _get_response( try: protected_path = attachment.protected_path( format_=request.accepted_renderer.format, - size=size, + suffix=suffix, ) except FFMpegException: raise serializers.ValidationError({ From b37ba0bab33b02da6dcca121e6677e0fa8abc2fd Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 28 May 2024 18:29:40 -0400 Subject: [PATCH 12/21] Handle encoded media path with S3 and FileSystem --- kpi/deployment_backends/kc_access/shadow_models.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/kpi/deployment_backends/kc_access/shadow_models.py b/kpi/deployment_backends/kc_access/shadow_models.py index 155804d03d..664ab1f535 100644 --- a/kpi/deployment_backends/kc_access/shadow_models.py +++ b/kpi/deployment_backends/kc_access/shadow_models.py @@ -181,25 +181,33 @@ def protected_path( else: attachment_file_path = self.absolute_path + optimized_image_path = None if suffix and self.mimetype.startswith('image/'): optimized_image_path = get_optimized_image_path( self.media_file.name, suffix ) if not default_kobocat_storage.exists(optimized_image_path): resize(self.media_file.name) - attachment_file_path = default_kobocat_storage.path(optimized_image_path) if isinstance(get_kobocat_storage(), KobocatFileSystemStorage): # Django normally sanitizes accented characters in file names during # save on disk but some languages have extra letters # (out of ASCII character set) and must be encoded to let NGINX serve # them + if optimized_image_path: + attachment_file_path = default_kobocat_storage.path( + optimized_image_path + ) protected_url = urlquote(attachment_file_path.replace( settings.KOBOCAT_MEDIA_ROOT, '/protected') ) else: # Double-encode the S3 URL to take advantage of NGINX's # otherwise troublesome automatic decoding + if optimized_image_path: + attachment_file_path = default_kobocat_storage.url( + optimized_image_path + ) protected_url = f'/protected-s3/{urlquote(attachment_file_path)}' return protected_url From 0c772857726b16d191178e5d0a35db1cf141567c Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 28 May 2024 19:11:07 -0400 Subject: [PATCH 13/21] Make Gallery use the small picture --- jsapp/js/components/formGallery/formGallery.component.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsapp/js/components/formGallery/formGallery.component.tsx b/jsapp/js/components/formGallery/formGallery.component.tsx index 44ea402ebc..44dc61b60c 100644 --- a/jsapp/js/components/formGallery/formGallery.component.tsx +++ b/jsapp/js/components/formGallery/formGallery.component.tsx @@ -167,7 +167,7 @@ export default function FormGallery(props: FormGalleryProps) { target='_blank' > {attachment.filename} Date: Wed, 29 May 2024 15:15:29 -0400 Subject: [PATCH 14/21] Remove CSV imports from Kobocat API v1 --- .../api/tests/viewsets/test_xform_viewset.py | 207 +----------------- .../apps/api/viewsets/xform_viewset.py | 47 ---- .../openrosa/libs/tests/utils/__init__.py | 1 - .../libs/tests/utils/test_csv_import.py | 77 ------- kobo/apps/openrosa/libs/utils/csv_import.py | 141 ------------ 5 files changed, 4 insertions(+), 469 deletions(-) delete mode 100644 kobo/apps/openrosa/libs/tests/utils/__init__.py delete mode 100644 kobo/apps/openrosa/libs/tests/utils/test_csv_import.py delete mode 100644 kobo/apps/openrosa/libs/utils/csv_import.py diff --git a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py index 6b5fb23a37..2831e70fb3 100644 --- a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py +++ b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py @@ -1,7 +1,6 @@ # coding: utf-8 import os import re -from io import StringIO from xml.dom import Node from django.conf import settings @@ -12,13 +11,12 @@ from kobo_service_account.utils import get_request_headers from rest_framework import status -from kobo.apps.openrosa.apps.api.tests.viewsets.test_abstract_viewset import \ - TestAbstractViewSet +from kobo.apps.openrosa.apps.api.tests.viewsets.test_abstract_viewset import ( + TestAbstractViewSet, +) from kobo.apps.openrosa.apps.api.viewsets.xform_viewset import XFormViewSet -from kobo.apps.openrosa.apps.logger.models import XForm, Instance +from kobo.apps.openrosa.apps.logger.models import XForm from kobo.apps.openrosa.libs.constants import ( - CAN_ADD_SUBMISSIONS, - CAN_CHANGE_XFORM, CAN_VIEW_XFORM ) from kobo.apps.openrosa.libs.serializers.xform_serializer import XFormSerializer @@ -484,203 +482,6 @@ def test_xform_serializer_none(self): } self.assertEqual(data, XFormSerializer(None).data) - def test_csv_import(self): - self.publish_xls_form() - view = XFormViewSet.as_view({'post': 'csv_import'}) - csv_import = open( - os.path.join( - settings.OPENROSA_APP_DIR, - 'libs', - 'tests', - 'fixtures', - 'good.csv', - ) - ) - post_data = {'csv_file': csv_import} - request = self.factory.post('/', data=post_data, **self.extra) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data.get('additions'), 9) - self.assertEqual(response.data.get('updates'), 0) - - def test_csv_import_fail(self): - self.publish_xls_form() - view = XFormViewSet.as_view({'post': 'csv_import'}) - csv_import = open(os.path.join(settings.OPENROSA_APP_DIR, 'libs', - 'tests', 'fixtures', 'bad.csv')) - post_data = {'csv_file': csv_import} - request = self.factory.post('/', data=post_data, **self.extra) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIsNotNone(response.data.get('error')) - - def test_csv_import_fail_invalid_field_post(self): - """ - Test that invalid post returns 400 with the error in json response - """ - self.publish_xls_form() - view = XFormViewSet.as_view({'post': 'csv_import'}) - csv_import = open(os.path.join(settings.OPENROSA_APP_DIR, 'libs', - 'tests', 'fixtures', 'bad.csv')) - post_data = {'wrong_file_field': csv_import} - request = self.factory.post('/', data=post_data, **self.extra) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIsNotNone(response.data.get('error')) - - def test_csv_import_fail_anonymous(self): - self.publish_xls_form() - view = XFormViewSet.as_view({'post': 'csv_import'}) - csv_import = open(os.path.join(settings.OPENROSA_APP_DIR, 'libs', - 'tests', 'fixtures', 'good.csv')) - post_data = {'csv_file': csv_import} - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data=post_data - ) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - - def test_csv_import_fail_non_owner(self): - self.publish_xls_form() - self._make_submissions() - - # Retain Bob's credentials for later use - bob_request_extra = self.extra - - # Switch to a privileged but non-owning user - self._login_user_and_profile( - extra_post_data={ - 'username': 'alice', - 'email': 'alice@localhost.com', - } - ) - self.assertEqual(self.user.username, 'alice') - assign_perm(CAN_VIEW_XFORM, self.user, self.xform) - assign_perm(CAN_CHANGE_XFORM, self.user, self.xform) - assign_perm(CAN_ADD_SUBMISSIONS, self.user, self.xform) - - # Surprise: `meta/instanceID` is ignored; `_uuid` is what's examined by - # the CSV importer to determine whether or not a row updates (edits) an - # existing submission - bob_instance = self.xform.instances.first() - edit_csv_bob = ( - 'formhub/uuid,_uuid,transport/available_transportation_types_to_referral_facility\n' - f'{self.xform.uuid},{bob_instance.uuid},boo!' - ) - - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data={'csv_file': StringIO(edit_csv_bob)}, - **self.extra - ) - view = XFormViewSet.as_view({'post': 'csv_import'}) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - # Check that we are sane and that Bob can edit their own submissions - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data={'csv_file': StringIO(edit_csv_bob)}, - **bob_request_extra - ) - view = XFormViewSet.as_view({'post': 'csv_import'}) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_200_OK) - bob_instance_old_uuid = bob_instance.uuid - bob_instance.refresh_from_db() - self.assertEqual( - bob_instance.json[ - 'transport/available_transportation_types_to_referral_facility' - ], - 'boo!' - ) - self.assertEqual( - bob_instance.json[ - 'meta/deprecatedID' - ], - f'uuid:{bob_instance_old_uuid}' - ) - - def test_csv_import_fail_edit_unauthorized_submission(self): - view = XFormViewSet.as_view({'post': 'csv_import'}) - - # Publish a form as Bob - self.publish_xls_form() - self._make_submissions() - bob_instance = self.xform.instances.first() - - # Publish another form as Alice - self._login_user_and_profile( - extra_post_data={ - 'username': 'alice', - 'email': 'alice@localhost.com', - } - ) - self.assertEqual(self.user.username, 'alice') - self.publish_xls_form() - self.assertEqual(self.xform.user.username, 'alice') - - # Make a submission, but not using `self._make_submissions()` because - # that allows for only one XForm at a time - new_csv_alice = ( - 'formhub/uuid,transport/available_transportation_types_to_referral_facility\n' - f'{self.xform.uuid},alice unedited' - ) - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data={'csv_file': StringIO(new_csv_alice)}, - **self.extra - ) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - # Verify that Alice can edit their own submissions - alice_instance = self.xform.instances.first() - edit_csv_alice = ( - 'formhub/uuid,_uuid,transport/available_transportation_types_to_referral_facility\n' - f'{self.xform.uuid},{alice_instance.uuid},alice edited' - ) - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data={'csv_file': StringIO(edit_csv_alice)}, - **self.extra - ) - response = view(request, pk=self.xform.id) - self.assertEqual(response.status_code, status.HTTP_200_OK) - alice_instance.refresh_from_db() - self.assertEqual( - alice_instance.json[ - 'transport/available_transportation_types_to_referral_facility' - ], - 'alice edited' - ) - - # Attempt to edit Bob's submission as Alice - original_bob_xml = bob_instance.xml - edit_csv_bob = ( - 'formhub/uuid,_uuid,transport/available_transportation_types_to_referral_facility\n' - f'{self.xform.uuid},{bob_instance.uuid},where does this go?!' - ) - request = self.factory.post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}), - data={'csv_file': StringIO(edit_csv_bob)}, - **self.extra - ) - response = view(request, pk=self.xform.id) - - # The attempted edit should appear as a new submission in Alice's form - # with the form and instance UUIDs overwritten - self.assertEqual(response.status_code, status.HTTP_200_OK) - bob_instance.refresh_from_db() - self.assertEqual(bob_instance.xml, original_bob_xml) - found_instance = Instance.objects.get( - xml__contains='where does this go?!' - ) - self.assertEqual(found_instance.xform, alice_instance.xform) - self.assertNotEqual(found_instance.xform, bob_instance.xform) - self.assertNotEqual(found_instance.uuid, bob_instance.uuid) - def test_cannot_publish_id_string_starting_with_number(self): data = { 'owner': self.user.username, diff --git a/kobo/apps/openrosa/apps/api/viewsets/xform_viewset.py b/kobo/apps/openrosa/apps/api/viewsets/xform_viewset.py index 4038f29b5a..cf9cfa49c1 100644 --- a/kobo/apps/openrosa/apps/api/viewsets/xform_viewset.py +++ b/kobo/apps/openrosa/apps/api/viewsets/xform_viewset.py @@ -28,7 +28,6 @@ from kobo.apps.openrosa.libs.renderers import renderers from kobo.apps.openrosa.libs.serializers.xform_serializer import XFormSerializer from kobo.apps.openrosa.libs.utils.common_tags import SUBMISSION_TIME -from kobo.apps.openrosa.libs.utils.csv_import import submit_csv from kobo.apps.openrosa.libs.utils.export_tools import ( generate_export, should_create_new_export, @@ -532,28 +531,6 @@ class XFormViewSet( > Response > > HTTP 200 OK - - ## Import CSV data to existing form - - - `csv_file` a valid csv file with exported \ - data (instance/submission per row) - - 
-    GET /api/v1/forms/{pk}/csv_import
-
- - > Example - > - > curl -X POST https://example.com/api/v1/forms/123/csv_import \ - -F csv_file=@/path/to/csv_import.csv - > - > Response - > - > HTTP 200 OK - > { - > "additions": 9, - > "updates": 0 - > } """ renderer_classes = api_settings.DEFAULT_RENDERER_CLASSES + [ renderers.XLSRenderer, @@ -655,30 +632,6 @@ def retrieve(self, request, *args, **kwargs): return custom_response_handler(request, xform, query, export_type) - @action(detail=True, methods=['POST']) - def csv_import(self, request, *args, **kwargs): - """ - Endpoint for CSV data imports - - Calls :py:func:`kobo.apps.openrosa.libs.utils.csv_import.submit_csv` - passing with the `request.FILES.get('csv_file')` upload for import. - """ - xform = self.get_object() - if request.user != xform.user: - # Access control for this endpoint previously relied on testing - # that the user had `logger.add_xform` on this specific XForm, - # which is meaningless but does get assigned to the XForm owner by - # the post-save signal handler - # `kobo.apps.openrosa.apps.logger.models.xform.set_object_permissions()`. - # For safety and clarity, this endpoint now explicitly denies - # access to all non-owners. - raise exceptions.PermissionDenied - resp = submit_csv(request, xform, request.FILES.get('csv_file')) - return Response( - data=resp, - status=status.HTTP_200_OK if resp.get('error') is None else - status.HTTP_400_BAD_REQUEST) - def perform_destroy(self, instance): username = instance.user.username xform_uuid = instance.uuid diff --git a/kobo/apps/openrosa/libs/tests/utils/__init__.py b/kobo/apps/openrosa/libs/tests/utils/__init__.py deleted file mode 100644 index 57d631c3f0..0000000000 --- a/kobo/apps/openrosa/libs/tests/utils/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# coding: utf-8 diff --git a/kobo/apps/openrosa/libs/tests/utils/test_csv_import.py b/kobo/apps/openrosa/libs/tests/utils/test_csv_import.py deleted file mode 100644 index 0426076e93..0000000000 --- a/kobo/apps/openrosa/libs/tests/utils/test_csv_import.py +++ /dev/null @@ -1,77 +0,0 @@ -# coding: utf-8 -import os -from io import StringIO, BytesIO - -from django.conf import settings -from django.test import RequestFactory -from django.urls import reverse - -from kobo.apps.openrosa.libs.utils import csv_import -from kobo.apps.openrosa.apps.logger.models import XForm -from kobo.apps.openrosa.apps.logger.models import Instance -from kobo.apps.openrosa.apps.main.tests.test_base import TestBase - - -class CSVImportTestCase(TestBase): - - def setUp(self): - super().setUp() - self.fixtures_dir = os.path.join( - settings.OPENROSA_APP_DIR, 'libs', 'tests', 'fixtures' - ) - self.good_csv = open(os.path.join(self.fixtures_dir, 'good.csv'), 'rb') - self.bad_csv = open(os.path.join(self.fixtures_dir, 'bad.csv'), 'rb') - xls_file_path = os.path.join(self.fixtures_dir, 'tutorial.xls') - self._publish_xls_file(xls_file_path) - self.xform = XForm.objects.get() - self.request = RequestFactory().post( - reverse('xform-csv-import', kwargs={'pk': self.xform.pk}) - ) - self.request.user = self.xform.user - - def test_submit_csv_param_sanity_check(self): - resp = csv_import.submit_csv(self.request, self.xform, 123456) - self.assertIsNotNone(resp.get('error')) - - # @mock.patch('kobo.apps.openrosa.libs.utils.csv_import.safe_create_instance') - # def test_submit_csv_xml_params(self, safe_create_instance): - # safe_create_instance.return_value = [None, {}] - # single_csv = open(os.path.join(self.fixtures_dir, 'single.csv')) - # csv_import.submit_csv(self.user.username, self.xform, single_csv) - # xml_file_param = StringIO(open(os.path.join(self.fixtures_dir, - # 'single.csv')).read()) - # safe_create_instance.assert_called_with(self.user.username, - # xml_file_param, [], - # self.xform.uuid, None) - - def test_submit_csv_and_rollback(self): - count = Instance.objects.count() - csv_import.submit_csv(self.request, self.xform, self.good_csv) - self.assertEqual(Instance.objects.count(), - count + 9, 'submit_csv test Failed!') - # CSV imports previously allowed `_submitted_by` to be set arbitrarily; - # it is now always assigned to the user requesting the CSV import - self.assertEqual( - Instance.objects.filter(user=self.user).count(), count + 9 - ) - - def test_submit_csv_edits(self): - csv_import.submit_csv(self.request, self.xform, self.good_csv) - self.assertEqual( - Instance.objects.count(), 9, 'submit_csv edits #1 test Failed!' - ) - - edit_csv = open(os.path.join(self.fixtures_dir, 'edit.csv'), 'rb') - edit_csv_str = edit_csv.read().decode() - - edit_csv = BytesIO( - edit_csv_str.format( - *[x.get('uuid') for x in Instance.objects.values('uuid')] - ).encode() - ) - - count = Instance.objects.count() - csv_import.submit_csv(self.request, self.xform, edit_csv) - self.assertEqual( - Instance.objects.count(), count, 'submit_csv edits #2 test Failed!' - ) diff --git a/kobo/apps/openrosa/libs/utils/csv_import.py b/kobo/apps/openrosa/libs/utils/csv_import.py deleted file mode 100644 index 57b33b65ad..0000000000 --- a/kobo/apps/openrosa/libs/utils/csv_import.py +++ /dev/null @@ -1,141 +0,0 @@ -# coding: utf-8 -import csv -import io -import json -import uuid -from datetime import datetime -from typing import TextIO, Union - -from kobo.apps.openrosa.apps.logger.models import Instance -from kobo.apps.openrosa.libs.utils.logger_tools import ( - dict2xml, - safe_create_instance, -) - - -def get_submission_meta_dict(xform, instance_id): - """Generates metadata for our submission - - Checks if `instance_id` belongs to an existing submission. - If it does, it's considered an edit and its uuid gets deprecated. - In either case, a new one is generated and assigned. - - :param kobo.apps.openrosa.apps.logger.models.XForm xform: The submission's XForm. - :param string instance_id: The submission/instance `uuid`. - - :return: The metadata dict - :rtype: dict - """ - uuid_arg = 'uuid:{}'.format(uuid.uuid4()) - meta = {'instanceID': uuid_arg} - - update = 0 - - if xform.instances.filter(uuid=instance_id).count() > 0: - uuid_arg = 'uuid:{}'.format(uuid.uuid4()) - meta.update({'instanceID': uuid_arg, - 'deprecatedID': 'uuid:{}'.format(instance_id)}) - update += 1 - return [meta, update] - - -def dict2xmlsubmission(submission_dict, xform, instance_id, submission_date): - """Creates and xml submission from an appropriate dict (& other data) - - :param dict submission_dict: A dict containing form submission data. - :param kobo.apps.openrosa.apps.logger.models.XForm xfrom: The submission's XForm. - :param string instance_id: The submission/instance `uuid`. - :param string submission_date: An isoformatted datetime string. - - :return: An xml submission string - :rtype: string - """ - return ('' - '<{0} id="{1}" instanceID="uuid:{2}" submissionDate="{3}" ' - 'xmlns="http://opendatakit.org/submissions">{4}' - ''.format( - json.loads(xform.json).get('name', xform.id_string), - xform.id_string, instance_id, submission_date, - dict2xml(submission_dict).replace('\n', ''))) - - -def submit_csv( - request: 'django.http.HttpRequest', - xform: 'kobo.apps.openrosa.apps.logger.models.XForm', - csv_file: Union[str, TextIO], -) -> dict: - """ - Imports CSV data to an existing form - - Takes a csv formatted file or string containing rows of submission/instance - and converts those to xml submissions and finally submits them by calling - :py:func:`kobo.apps.openrosa.libs.utils.logger_tools.safe_create_instance` - - """ - - if hasattr(csv_file, 'readable'): - csv_file = io.TextIOWrapper(csv_file, encoding='utf-8') - - if isinstance(csv_file, str): - csv_file = io.StringIO(csv_file) - elif csv_file is None or not hasattr(csv_file, 'read'): - return { - 'error': ( - 'Invalid param type for `csv_file`. ' - 'Expected file or String ' - 'got {} instead.'.format(type(csv_file).__name__) - ) - } - - csv_reader = csv.DictReader(csv_file) - rollback_uuids = [] - submission_time = datetime.utcnow().isoformat() - ona_uuid = {'formhub': {'uuid': xform.uuid}} - error = None - additions = inserts = 0 - for row in csv_reader: - # fetch submission uuid before purging row metadata - row_uuid = row.get('_uuid') - submission_date = row.get('_submission_time', submission_time) - - row_iter = dict(row) - for key in row_iter: # seems faster than a comprehension - # remove metadata (keys starting with '_') - if key.startswith('_'): - del row[key] - # process nested data e.g x[formhub/uuid] => x[formhub][uuid] - if r'/' in key: - p, c = key.split('/') - row[p] = {c: row[key]} - del row[key] - - # inject our form's uuid into the submission - row.update(ona_uuid) - - old_meta = row.get('meta', {}) - new_meta, update = get_submission_meta_dict(xform, row_uuid) - inserts += update - old_meta.update(new_meta) - row.update({'meta': old_meta}) - - row_uuid = row.get('meta').get('instanceID') - rollback_uuids.append(row_uuid.replace('uuid:', '')) - - xml_file = io.StringIO( - dict2xmlsubmission(row, xform, row_uuid, submission_date)) - - try: - error, instance = safe_create_instance( - request.user.username, xml_file, [], xform.uuid, request - ) - except ValueError as e: - error = e - - if error: - Instance.objects.filter(uuid__in=rollback_uuids, - xform=xform).delete() - return {'error': str(error)} - else: - additions += 1 - - return {'additions': additions - inserts, 'updates': inserts} From 1d286e88d118000dc26a360298990ce6cbbd0422 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Wed, 29 May 2024 15:15:48 -0400 Subject: [PATCH 15/21] Update README --- README.md | 12 +- kobo/apps/openrosa/README.md | 115 +++++++++++++++ kobo/apps/openrosa/REMOVALS.md | 256 +++++++++++++++++++++++++++++++++ 3 files changed, 381 insertions(+), 2 deletions(-) create mode 100644 kobo/apps/openrosa/README.md create mode 100644 kobo/apps/openrosa/REMOVALS.md diff --git a/README.md b/README.md index e1039dd3b2..ca41285963 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,17 @@ We're open for [contributions](./CONTRIBUTING.md)! +## Important notice when upgrading from any release older than [`2.024.19`](https://github.com/kobotoolbox/kpi/releases/tag/2.024.19) + +Prior to release [`2.024.19`](https://github.com/kobotoolbox/kpi/releases/tag/2.024.19), this project (KPI) and [KoboCAT](https://github.com/kobotoolbox/kobocat) were two separated projects. +KoboCAT is now part of KPI code base and its repository has been archived. + +KoboCAT deprecation notices will be maintained in this repository. +[More details here](./kobo/apps/openrosa/README.md) + ## Important notice when upgrading from any release older than [`2.020.18`](https://github.com/kobotoolbox/kpi/releases/tag/2.020.18) -Prior to release [`2.020.18`](https://github.com/kobotoolbox/kpi/releases/tag/2.020.18), this project (KPI) and [KoBoCAT](https://github.com/kobotoolbox/kobocat) both shared a common Postgres database. They now each have their own. **If you are upgrading an existing single-database installation, you must follow [these instructions](https://community.kobotoolbox.org/t/upgrading-to-separate-databases-for-kpi-and-kobocat/7202)** to migrate the KPI tables to a new database and adjust your configuration appropriately. +Prior to release [`2.020.18`](https://github.com/kobotoolbox/kpi/releases/tag/2.020.18), this project (KPI) and [KoboCAT](https://github.com/kobotoolbox/kobocat) both shared a common Postgres database. They now each have their own. **If you are upgrading an existing single-database installation, you must follow [these instructions](https://community.kobotoolbox.org/t/upgrading-to-separate-databases-for-kpi-and-kobocat/7202)** to migrate the KPI tables to a new database and adjust your configuration appropriately. If you do not want to upgrade at this time, please use the [`shared-database-obsolete`](https://github.com/kobotoolbox/kpi/tree/shared-database-obsolete) branch instead. @@ -35,7 +43,7 @@ syntax, see the documentation at the top of ## Admin reports -There are several types of data reports available to superusers. +There are several types of data reports available to superusers. * Full list of users including their details provided during signup, number of deployed projects (XForm count), number of submissions, date joined, and last login: `/superuser_stats/user_report/`. File being created is a CSV, so don't download immediately to wait for server to be finished writing to the file (it will download even if incomplete). * Monthly aggregate figures for number of forms, deployed projects, and submissions (from kobocat): `//superuser_stats/` diff --git a/kobo/apps/openrosa/README.md b/kobo/apps/openrosa/README.md new file mode 100644 index 0000000000..dadf8b4b5a --- /dev/null +++ b/kobo/apps/openrosa/README.md @@ -0,0 +1,115 @@ +# KoboCAT + +## Important notice when upgrading from any release older than [`2.020.18`](https://github.com/kobotoolbox/kobocat/releases/tag/2.020.18) + +Up to and including release [`2.020.18`](https://github.com/kobotoolbox/kobocat/releases/tag/2.020.18), this project (KoboCAT) and [KPI](https://github.com/kobotoolbox/kpi) both shared a common Postgres database. They now each have their own. **If you are upgrading an existing single-database installation, you must follow [these instructions](https://community.kobotoolbox.org/t/upgrading-to-separate-databases-for-kpi-and-kobocat/7202)** to migrate the KPI tables to a new database and adjust your configuration appropriately. + +If you do not want to upgrade at this time, please use the [`shared-database-obsolete`](https://github.com/kobotoolbox/kobocat/tree/shared-database-obsolete) branch instead. + +## Deprecation Notices + +Much of the user-facing features of this application are being migrated +to . KoboCAT's data-access API and +OpenRosa functions will remain intact, and any plans to the contrary +will be announced well in advance. For more details and discussion, +please refer to +. + +As features are migrated, we will list them here along with the last +release where each was present: + - Starting from version [2.024.19](https://github.com/kobotoolbox/kobocat/releases/tag/2.024.19), + the ability to import submissions in CSV to KoboCAT was + removed (i.e: `https://kobocat.domain.tld/api/v1/forms//csv_import`) + . + - On 14 June 2021, the ability to upload forms directly to KoboCAT was + removed, and it was announced that the legacy KoboCAT user interface would + be preserved for "a few more months". After more than two years, we have + removed the user interface and related endpoints entirely in release + [2.023.37](https://github.com/kobotoolbox/kobocat/releases/tag/2.023.37). + **This includes the ability to upload XLSForms via the legacy KoboCAT API.** + Please use the KPI `v2` API for all form management. Other removed features + should already be available in KPI as well. Please see + [REMOVALS.md](REMOVALS.md) for a complete list. + - To ensure security and stability, many endpoints that were already + available in KPI, long-unsupported, or underutilized have been removed in + release + [2.020.40](https://github.com/kobotoolbox/kobocat/releases/tag/2.020.40). + These were related to charts and stats, form cloning, form sharing, user + profiles, organizations / projects / teams, bamboo, and ziggy. For a full + list, please see [REMOVALS.md](REMOVALS.md). These endpoints were last + available in the release + [2.020.39](https://github.com/kobotoolbox/kobocat/releases/tag/2.020.39). + - REST Services - an improved version [has been added to + KPI](https://github.com/kobotoolbox/kpi/pull/1864). The last KoboCAT + release to contain legacy REST services is + [2.019.39](https://github.com/kobotoolbox/kobocat/releases/tag/2.019.39). + +## About + +kobocat is the data collection platform used in KoboToolbox. It is based +on the excellent [onadata](http://github.com/onaio/onadata) platform +developed by Ona LLC, which in itself is a redevelopment of the +[formhub](http://github.com/SEL-Columbia/formhub) platform developed by +the Sustainable Engineering Lab at Columbia University. + +Please refer to +[kobo-install](https://github.com/kobotoolbox/kobo-install) for +instructions on how to install KoboToolbox. + +## Code Structure + + - **logger** - This app serves XForms to and receives submissions from + ODK Collect and Enketo. + - **viewer** - This app provides a csv and xls export of the data + stored in logger. This app uses a data dictionary as produced by + pyxform. It also provides a map and single survey view. + - **main** - This app is the glue that brings logger and viewer + together. + +## Localization + +To generate a locale from scratch (ex. Spanish) + +``` sh +$ django-admin makemessages -l es -e py,html,email,txt ; +$ for app in {main,viewer} ; do cd kobocat/apps/${app} && django-admin makemessages -d djangojs -l es && cd - ; done +``` + +To update PO files + +``` sh +$ django-admin makemessages -a ; +$ for app in {main,viewer} ; do cd kobocat/apps/${app} && django-admin makemessages -d djangojs -a && cd - ; done +``` + +To compile MO files and update live translations + +``` sh +$ django-admin compilemessages ; +$ for app in {main,viewer} ; do cd kobocat/apps/${app} && django-admin compilemessages && cd - ; done +``` +## Testing in KoboCAT + +For kobo-install users, enter the folder for kobo-install and run this command + +``` +./run.py -cf exec kobocat bash +``` + +For all other users, enter the container using this command + +``` sh +$ docker exec -it {{kobocat container}} /bin/bash +``` + +Run pip install the development dependencies + +``` sh +$ pip install -r dependencies/pip/dev.txt +``` + +Run pytest to run all automated tests + +``` sh +$ pytest +``` diff --git a/kobo/apps/openrosa/REMOVALS.md b/kobo/apps/openrosa/REMOVALS.md new file mode 100644 index 0000000000..b537cda98d --- /dev/null +++ b/kobo/apps/openrosa/REMOVALS.md @@ -0,0 +1,256 @@ +# KoboCAT endpoint removals as of release 2.023.37 + +The entire KoboCAT user interface has been removed. +The last release to contain a user interface or any of the endpoints listed below was [2.023.21](https://github.com/kobotoolbox/kpi/releases/tag/2.023.21). + +## Obsolete User Interface + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/` | `onadata.apps.main.views.home` +`//` | `onadata.apps.main.views.profile` | `user_profile` +`//api-token` | `onadata.apps.main.views.api_token` | `api_token` + +## Obsolete KPI Integrations + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/login_redirect/` | `onadata.apps.main.views.login_redirect` +`//forms//map` | `onadata.apps.main.views.view_func` | `redirect_map_to_kpi` +`//reports//digest.html` | `onadata.apps.main.views.view_func` | `redirect_analyze_data_to_kpi` +`//reports//export.html` | `onadata.apps.main.views.view_func` | `redirect_view_data_in_table_to_kpi` + +## Obsolete Enketo Integration + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/forms//enketo\./` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-enketo` +`/api/v1/forms//enketo` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-enketo` + +## Obsolete Form Management + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/forms/` | `onadata.apps.main.views.show` | `show_form` +`//forms//api` | `onadata.apps.main.views.api` | `mongo_view_api` +`//forms//data\.csv` | `onadata.apps.viewer.views.data_export` | `csv_export` +`//forms//data\.kml` | `onadata.apps.viewer.views.kml_export` +`//forms//data\.xls` | `onadata.apps.viewer.views.data_export` | `xls_export` +`//forms//delete-doc/` | `onadata.apps.main.views.delete_metadata` | `delete_metadata` +`//forms//doc/` | `onadata.apps.main.views.download_metadata` | `download_metadata` +`//forms//edit` | `onadata.apps.main.views.edit` | `edit_form` +`//forms//formid-media/` | `onadata.apps.main.views.download_media_data` | `download_media_data` +`//forms//form_settings` | `onadata.apps.main.views.show_form_settings` | `show_form_settings` +`//forms/` | `onadata.apps.main.views.show` | `show_form` +`//forms//photos` | `onadata.apps.main.views.form_photos` | `form_photos` +`//superuser_stats/` | `onadata.apps.logger.views.retrieve_superuser_stats` +`//superuser_stats/` | `onadata.apps.logger.views.superuser_stats` + +## Obsolete Documentation + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/admin/doc/bookmarklets/` | `django.contrib.admindocs.views.BookmarkletsView` | `django-admindocs-bookmarklets` +`/admin/doc/` | `django.contrib.admindocs.views.BaseAdminDocsView` | `django-admindocs-docroot` +`/admin/doc/filters/` | `django.contrib.admindocs.views.TemplateFilterIndexView` | `django-admindocs-filters` +`/admin/doc/models/\./` | `django.contrib.admindocs.views.ModelDetailView` | `django-admindocs-models-detail` +`/admin/doc/models/` | `django.contrib.admindocs.views.ModelIndexView` | `django-admindocs-models-index` +`/admin/doc/tags/` | `django.contrib.admindocs.views.TemplateTagIndexView` | `django-admindocs-tags` +`/admin/doc/templates//` | `django.contrib.admindocs.views.TemplateDetailView` | `django-admindocs-templates` +`/admin/doc/views/` | `django.contrib.admindocs.views.ViewIndexView` | `django-admindocs-views-index` +`/admin/doc/views//` | `django.contrib.admindocs.views.ViewDetailView` | `django-admindocs-views-detail` +`/api-docs/` | `django.views.generic.base.RedirectView` + +# KoboCAT endpoint removals as of release 2.021.22 + +The last release to contain any of the endpoints listed below was [2.021.21](https://github.com/kobotoolbox/kpi/releases/tag/2.021.21). + +## Submission Data Access and Management + +URL Pattern | View Class or Function | Description | Available in KPI +-- | -- | -- | -- +`//exports/.csv` | `onadata.apps.export.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//exports/.html` | `onadata.apps.export.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//exports/.xlsx` | `onadata.apps.export.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//exports//` | `onadata.apps.export.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//forms//add-submission-with` | `onadata.apps.viewer.views.add_submission_with` | Unknown (no hits in access log, undocumented, no UI, no significant development since 2013) | No +`//forms//data.sav.zip` | `onadata.apps.viewer.views.data_export` | SPSS SAV data export (dropped from UI in 2015) | No +`//forms//delete_data` | `onadata.apps.main.views.delete_data` | Delete an individual submission | Yes +`//forms//edit-data/` | `onadata.apps.logger.views.edit_data` | Edit a submission in Enketo | Yes +`//forms//enter-data` | `onadata.apps.logger.views.enter_data` | Add a submission with Enketo | Yes +`//forms//instance` | `onadata.apps.viewer.views.instance` | View a single instance | Yes +`//forms//map` | `onadata.apps.viewer.views.map_view` | View submissions on a map | Yes +`//forms//map_embed` | `onadata.apps.viewer.views.map_embed_view` | View submissions on a map | Yes +`//forms//view-data` | `onadata.apps.viewer.views.data_view` | View submissions in a table | Yes +`//reports//digest.html` | `onadata.apps.survey_report.views._wrapper` | “Analyze data” (outdated version of KPI’s “Reports”) | Yes +`//reports//digest/` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//reports//export.csv` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//reports//export.html` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//reports//export.xlsx` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//reports//export/` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes +`//reports//submission/.html` | `onadata.apps.survey_report.views._wrapper` | Old version of formpack (not legacy) exports | Yes + +## Project Management + +URL Pattern | View Class or Function | Description | Available in KPI +-- | -- | -- | -- +`///toggle_downloadable/` | `onadata.apps.logger.views.toggle_downloadable` | Switch data collection on and off for a project | Yes, as “archive” +`//cloneform` | `onadata.apps.main.views.clone_xlsform` | Unused and undocumented form cloning feature | Yes +`//delete//` | `onadata.apps.logger.views.delete_xform` | Delete an entire project | Yes +`//forms//delete-doc/` | `onadata.apps.main.views.delete_metadata` | Delete background documents | No +`//forms//perms` | `onadata.apps.main.views.set_perm` | Set project permissions | Yes +`//forms//preview` | `onadata.apps.main.views.enketo_preview` | Preview a form in Enketo | Yes +`//forms//public_api` | `onadata.apps.main.views.public_api` | Basic form metadata: title, owner’s username, date created/modified, id_string, uuid, sharing status, archival status | Yes, but only if public access to the project is explicitly granted +`//forms//qrcode` | `onadata.apps.main.views.qrcode` | QR code (as a HTML page) containing Enketo data-entry URL | No +`//forms//update` | `onadata.apps.main.views.update_xform` | Overwrite a project with new XLSForm | Yes +`/api/v1/forms//enketo` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | Retrieve Enketo data entry URL | Yes + +## Already Unused or Non-functional + +URL Pattern | View Class or Function | Description | Available in KPI +-- | -- | -- | -- +`//activity` | `onadata.apps.main.views.activity` | Audit log entries (non-functional) | No +`//activity/api` | `onadata.apps.main.views.activity_api` | Audit log entries (non-functional) | No +`//forms//gdocs` | `onadata.apps.viewer.views.google_xls_export` | Google Docs export (non-functional) | No +`//forms//sms_multiple_submissions` | `onadata.apps.sms_support.views.import_multiple_submissions_for_form` | Data collection via SMS (non-functional) | No +`//forms//sms_submission` | `onadata.apps.sms_support.views.import_submission_for_form` | Data collection via SMS (non-functional) | No +`//forms//sms_submission//` | `onadata.apps.sms_support.providers.import_submission_for_form` | Data collection via SMS (non-functional) | No +`//forms//thank_you_submission` | `onadata.apps.viewer.views.thank_you_submission` | Data collection via SMS (non-functional) | No +`//sms_multiple_submissions` | `onadata.apps.sms_support.views.import_multiple_submissions` | Data collection via SMS (non-functional) | No +`//sms_submission` | `onadata.apps.sms_support.views.import_submission` | Data collection via SMS (non-functional) | No +`//sms_submission//` | `onadata.apps.sms_support.providers.import_submission` | Data collection via SMS (non-functional) | No +`/about-us/` | `onadata.apps.main.views.about_us` | Unused informational page | Yes (on kobotoolbox.org) +`/accounts/activate//` | `registration.backends.default.views.ActivationView` | Unused user management | Yes (always was) +`/accounts/activate/complete/` | `django.views.generic.base.TemplateView` | Unused user management | Yes (always was) +`/accounts/password/change/` | `django.contrib.auth.views.password_change` | Unused user management | Yes (always was) +`/accounts/password/change/done/` | `django.contrib.auth.views.password_change_done` | Unused user management | Yes (always was) +`/accounts/password/reset/` | `django.contrib.auth.views.password_reset` | Unused user management | Yes (always was) +`/accounts/password/reset/complete/` | `django.contrib.auth.views.password_reset_complete` | Unused user management | Yes (always was) +`/accounts/password/reset/confirm///` | `django.contrib.auth.views.password_reset_confirm` | Unused user management | Yes (always was) +`/accounts/password/reset/done/` | `django.contrib.auth.views.password_reset_done` | Unused user management | Yes (always was) +`/accounts/register/` | `onadata.apps.main.registration_views.FHRegistrationView` | Unused user management | Yes (always was) +`/accounts/register/complete/` | `django.views.generic.base.TemplateView` | Unused user management | Yes (always was) +`/activity/fields` | `onadata.apps.main.views.activity_fields` | Schema for audit log | No +`/api/v1/profiles//change_password` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | Unused user management | Yes (always was) +`/api/v1/profiles//change_password./` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | Unused user management | Yes (always was) +`/faq/` | `onadata.apps.main.views.faq` | Unused informational page | Yes (on kobotoolbox.org) +`/gauthtest/` | `onadata.apps.main.google_export.google_oauth2_request` | Google authentication (non-functional) | No +`/getting_started/` | `onadata.apps.main.views.getting_started` | Unused informational page | Yes (on kobotoolbox.org) +`/gwelcome/` | `onadata.apps.main.google_export.google_auth_return` | Google authentication (non-functional) | No +`/people/` | `onadata.apps.main.views.members_list` | Unused informational page | Yes (on kobotoolbox.org) +`/privacy/` | `onadata.apps.main.views.privacy` | Unused informational page | Yes +`/resources/` | `onadata.apps.main.views.resources` | Unused informational page | Yes (on kobotoolbox.org) +`/support/` | `onadata.apps.main.views.support` | Unused informational page | Yes (on kobotoolbox.org) +`/syntax/` | `onadata.apps.main.views.syntax` | Unused informational page | Yes (on kobotoolbox.org) +`/tos/` | `onadata.apps.main.views.tos` | Unused informational page | Yes +`/tutorial/` | `onadata.apps.main.views.tutorial` | Unused informational page | Yes (on kobotoolbox.org) +`/typeahead_usernames` | `onadata.apps.main.views.username_list` | Username autocompletion for legacy permissions UI (not API endpoint for external use) | No (removed for privacy) +`/xls2xform/` | `onadata.apps.main.views.xls2xform` | Unused informational page | Yes (on kobotoolbox.org) + + +# KoboCAT endpoint removals as of release [2.020.40](https://github.com/kobotoolbox/kobocat/releases/tag/2.020.40) + +The last release to contain any of the endpoints listed below was https://github.com/kobotoolbox/kobocat/releases/tag/2.020.39. + +## Already available in KPI + +### Charts and Stats + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`//forms//stats` | `onadata.apps.viewer.views.charts` | `form-stats` +`//forms//tables` | `onadata.apps.viewer.views.stats_tables` | +`/api/v1/charts` | `onadata.apps.api.viewsets.charts_viewset.ChartsViewSet` | `chart-list` +`/api/v1/charts./` | `onadata.apps.api.viewsets.charts_viewset.ChartsViewSet` | `chart-list` +`/api/v1/charts/` | `onadata.apps.api.viewsets.charts_viewset.ChartsViewSet` | `chart-detail` +`/api/v1/charts/./` | `onadata.apps.api.viewsets.charts_viewset.ChartsViewSet` | `chart-detail` +`/api/v1/stats` | `onadata.apps.api.viewsets.stats_viewset.StatsViewSet` | `stats-list` +`/api/v1/stats./` | `onadata.apps.api.viewsets.stats_viewset.StatsViewSet` | `stats-list` +`/api/v1/stats/` | `onadata.apps.api.viewsets.stats_viewset.StatsViewSet` | `stats-detail` +`/api/v1/stats/./` | `onadata.apps.api.viewsets.stats_viewset.StatsViewSet` | `stats-detail` +`/api/v1/stats/submissions` | `onadata.apps.api.viewsets.submissionstats_viewset.SubmissionStatsViewSet` | `submissionstats-list` +`/api/v1/stats/submissions./` | `onadata.apps.api.viewsets.submissionstats_viewset.SubmissionStatsViewSet` | `submissionstats-list` +`/api/v1/stats/submissions/` | `onadata.apps.api.viewsets.submissionstats_viewset.SubmissionStatsViewSet` | `submissionstats-detail` +`/api/v1/stats/submissions/./` | `onadata.apps.api.viewsets.submissionstats_viewset.SubmissionStatsViewSet` | `submissionstats-detail` +`/stats/` | `onadata.apps.stats.views.stats` | `form-stats` +`/stats/submissions/` | `onadata.apps.stats.views.submissions` | + +### Form Cloning + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/forms//clone` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-clone` +`/api/v1/forms//clone./` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-clone` + +### Form Sharing + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/forms//share` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-share` +`/api/v1/forms//share./` | `onadata.apps.api.viewsets.xform_viewset.XFormViewSet` | `xform-share` + +### User Profiles + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/profiles` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-list` +`/api/v1/profiles./` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-list` +`/api/v1/profiles/` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-detail` +`/api/v1/profiles/./` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-detail` +`/api/v1/profiles//change_password` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-change-password` +`/api/v1/profiles//change_password./` | `onadata.apps.api.viewsets.user_profile_viewset.UserProfileViewSet` | `userprofile-change-password` +`/api/v1/user/reset` | `onadata.apps.api.viewsets.connect_viewset.ConnectViewSet` | `userprofile-reset` +`/api/v1/user/reset./` | `onadata.apps.api.viewsets.connect_viewset.ConnectViewSet` | `userprofile-reset` +`/api/v1/users` | `onadata.apps.api.viewsets.user_viewset.UserViewSet` | `user-list` +`/api/v1/users./` | `onadata.apps.api.viewsets.user_viewset.UserViewSet` | `user-list` +`/api/v1/users/` | `onadata.apps.api.viewsets.user_viewset.UserViewSet` | `user-detail` +`/api/v1/users/./` | `onadata.apps.api.viewsets.user_viewset.UserViewSet` | `user-detail` + +## Discontinued + +These endpoints existed but were neither maintained nor tested, and their features were never available in the UI. They might be added to KPI at an indeterminate future time given interest and funding. + +### Organizations, Projects, and Teams + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/orgs` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-list` +`/api/v1/orgs./` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-list` +`/api/v1/orgs/` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-detail` +`/api/v1/orgs/./` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-detail` +`/api/v1/orgs//members` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-members` +`/api/v1/orgs//members./` | `onadata.apps.api.viewsets.organization_profile_viewset.OrganizationProfileViewSet` | `organizationprofile-members` +`/api/v1/projects` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-list` +`/api/v1/projects./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-list` +`/api/v1/projects/` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-detail` +`/api/v1/projects/./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-detail` +`/api/v1/projects//forms` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-forms` +`/api/v1/projects//forms./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-forms` +`/api/v1/projects//labels` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-labels` +`/api/v1/projects//labels./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-labels` +`/api/v1/projects//share` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-share` +`/api/v1/projects//share./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-share` +`/api/v1/projects//star` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-star` +`/api/v1/projects//star./` | `onadata.apps.api.viewsets.project_viewset.ProjectViewSet` | `project-star` +`/api/v1/teams` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-list` +`/api/v1/teams./` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-list` +`/api/v1/teams/` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-detail` +`/api/v1/teams/./` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-detail` +`/api/v1/teams//members` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-members` +`/api/v1/teams//members./` | `onadata.apps.api.viewsets.team_viewset.TeamViewSet` | `team-members` + +### User Profiles + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`/api/v1/user//starred` | `onadata.apps.api.viewsets.connect_viewset.ConnectViewSet` | `userprofile-starred` +`/api/v1/user//starred./` | `onadata.apps.api.viewsets.connect_viewset.ConnectViewSet` | `userprofile-starred` + +## Discontinued permanently + +### Bamboo and Ziggy + +URL Pattern | View Class or Function | View Name +-- | -- | -- +`//forms//bamboo` | `onadata.apps.main.views.link_to_bamboo` | +`//form-submissions` | `onadata.apps.logger.views.ziggy_submissions` | From 80bdb47944983b42c8eeac39831f530a8b1ab029 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 30 May 2024 14:05:14 -0400 Subject: [PATCH 16/21] Fix deleting kobocat form_disclaimer migration endlessly --- scripts/fix_migrations_for_kobocat_django_app.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/fix_migrations_for_kobocat_django_app.py b/scripts/fix_migrations_for_kobocat_django_app.py index 40c847d653..5cbd32395d 100644 --- a/scripts/fix_migrations_for_kobocat_django_app.py +++ b/scripts/fix_migrations_for_kobocat_django_app.py @@ -3,14 +3,15 @@ def run(): - migrate_custom_user_model() - delete_kobocat_form_disclaimer_app() + if migrate_custom_user_model(): + # Only run it when custom user model migrations have been fixed + delete_kobocat_form_disclaimer_app() def delete_kobocat_form_disclaimer_app(): """ KoboCAT form_disclaimer app does not exist anymore but its migrations - create conflicts and must be deleted before applying migration. + create conflicts and must be deleted before applying other migrations. """ with connections[settings.OPENROSA_DB_ALIAS].cursor() as kc_cursor: kc_cursor.execute( @@ -55,7 +56,9 @@ def migrate_custom_user_model(): "UPDATE django_content_type SET app_label = 'kobo_auth' " "WHERE app_label = 'auth' and model = 'user';" ) + return True else: print('Migration kobo_auth.0001 already applied. Skip!') + return False else: raise Exception('Run `./manage.py migrate auth` first') From 72264de040e98a33f5be623c087f46cbf1a512a0 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 30 May 2024 17:31:19 -0400 Subject: [PATCH 17/21] Fix migrations --- scripts/fix_migrations_for_kobocat_django_app.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/scripts/fix_migrations_for_kobocat_django_app.py b/scripts/fix_migrations_for_kobocat_django_app.py index 5cbd32395d..f8350d18cc 100644 --- a/scripts/fix_migrations_for_kobocat_django_app.py +++ b/scripts/fix_migrations_for_kobocat_django_app.py @@ -3,11 +3,27 @@ def run(): + if not are_migration_already_applied(): + return + if migrate_custom_user_model(): # Only run it when custom user model migrations have been fixed delete_kobocat_form_disclaimer_app() +def are_migration_already_applied(): + with connection.cursor() as cursor: + cursor.execute( + "SELECT EXISTS (" + " SELECT FROM pg_tables" + " WHERE schemaname = 'public'" + " AND tablename = 'django_migrations'" + ");" + ) + row = cursor.fetchone() + return bool(row[0]) + + def delete_kobocat_form_disclaimer_app(): """ KoboCAT form_disclaimer app does not exist anymore but its migrations From b4aba7b7be182b6fdd8e6ad61be7e8b1d1ebd1d5 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 30 May 2024 17:31:19 -0400 Subject: [PATCH 18/21] =?UTF-8?q?Fix=20race=20conditions=20related=20to=20?= =?UTF-8?q?shadow=20model=20writes=20=E2=80=A6made=20before=20running=20Ko?= =?UTF-8?q?bocat=20migrations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- kobo/apps/kobo_auth/models.py | 6 +- .../management/commands/delete_revisions.py | 8 +- .../commands/populate_submission_counters.py | 2 +- .../0015_add_delete_data_permission.py | 117 +++++++++--------- .../migrations/0011_drop_old_kpi_tables.py | 4 +- .../openrosa/apps/main/models/user_profile.py | 5 + kobo/apps/openrosa/apps/main/signals.py | 5 + kobo/apps/openrosa/libs/data/query.py | 4 +- kobo/settings/guardian.py | 8 ++ kpi/constants.py | 10 ++ kpi/db_routers.py | 5 +- .../kc_access/shadow_models.py | 17 ++- .../0055_set_require_auth_per_project.py | 13 +- kpi/signals.py | 3 +- scripts/create_anonymous_user.py | 9 ++ .../fix_migrations_for_kobocat_django_app.py | 16 +++ scripts/migrate.sh | 7 +- 17 files changed, 162 insertions(+), 77 deletions(-) create mode 100644 kobo/settings/guardian.py create mode 100644 scripts/create_anonymous_user.py diff --git a/kobo/apps/kobo_auth/models.py b/kobo/apps/kobo_auth/models.py index e63d20bbb9..b9b0151a7e 100644 --- a/kobo/apps/kobo_auth/models.py +++ b/kobo/apps/kobo_auth/models.py @@ -17,11 +17,11 @@ class Meta: swappable = 'AUTH_USER_MODEL' def has_perm(self, perm, obj=None): - # If it is a Kobocat permissions, check permission in Kobocat DB first + # If it is a KoboCAT permissions, check permission in KoboCAT DB first # 3 options: - # - `obj` is not None and its app_label belongs to Kobocat + # - `obj` is not None and its app_label belongs to KoboCAT # - `perm` format is ., we check the app label - # - `perm` belongs to Kobocat permission codenames + # - `perm` belongs to KoboCAT permission codenames if obj: if obj._meta.app_label in OPENROSA_APP_LABELS: with use_db(settings.OPENROSA_DB_ALIAS): diff --git a/kobo/apps/openrosa/apps/logger/management/commands/delete_revisions.py b/kobo/apps/openrosa/apps/logger/management/commands/delete_revisions.py index 29b1267a47..cb39a5c61b 100644 --- a/kobo/apps/openrosa/apps/logger/management/commands/delete_revisions.py +++ b/kobo/apps/openrosa/apps/logger/management/commands/delete_revisions.py @@ -2,7 +2,8 @@ from datetime import timedelta import sys -from django.db import transaction, models, router, connection +from django.conf import settings +from django.db import transaction, models, router, connections from django.utils import timezone from reversion.models import Revision, Version from reversion.management.commands.deleterevisions import Command as RevisionCommand @@ -137,7 +138,9 @@ def handle(self, *app_labels, **options): print("Done!") - def _do_vacuum(self, full=False): + @staticmethod + def _do_vacuum(full=False): + connection = connections[settings.OPENROSA_DB_ALIAS] cursor = connection.cursor() if full: print("Vacuuming (full) table {}...".format(Revision._meta.db_table)) @@ -150,4 +153,3 @@ def _do_vacuum(self, full=False): print("Vacuuming table {}...".format(Version._meta.db_table)) cursor.execute("VACUUM {}".format(Version._meta.db_table)) connection.commit() - diff --git a/kobo/apps/openrosa/apps/logger/management/commands/populate_submission_counters.py b/kobo/apps/openrosa/apps/logger/management/commands/populate_submission_counters.py index 5e9e173b37..cbaf2f0142 100644 --- a/kobo/apps/openrosa/apps/logger/management/commands/populate_submission_counters.py +++ b/kobo/apps/openrosa/apps/logger/management/commands/populate_submission_counters.py @@ -83,7 +83,7 @@ def handle(self, *args, **kwargs): ) for user in ( - User.objects.only('username') + User.objects.using(settings.OPENROSA_DB_ALIAS).only('username') .exclude(pk=settings.ANONYMOUS_USER_ID) .exclude(pk__in=subquery) .iterator(chunk_size=self._chunks) diff --git a/kobo/apps/openrosa/apps/logger/migrations/0015_add_delete_data_permission.py b/kobo/apps/openrosa/apps/logger/migrations/0015_add_delete_data_permission.py index 6cf35d5901..3635d7a809 100644 --- a/kobo/apps/openrosa/apps/logger/migrations/0015_add_delete_data_permission.py +++ b/kobo/apps/openrosa/apps/logger/migrations/0015_add_delete_data_permission.py @@ -1,9 +1,12 @@ # coding: utf-8 import sys -from django.db import migrations +from django.conf import settings from django.contrib.auth.management import create_permissions from django.contrib.auth.models import AnonymousUser +from django.db import migrations + +from kpi.utils.database import use_db def create_new_perms(apps): @@ -14,7 +17,9 @@ def create_new_perms(apps): """ for app_config in apps.get_app_configs(): app_config.models_module = True - create_permissions(app_config, apps=apps, verbosity=0) + create_permissions( + app_config, apps=apps, verbosity=0, using=settings.OPENROSA_DB_ALIAS + ) app_config.models_module = None @@ -26,27 +31,28 @@ def grant_model_level_perms(apps): Permission = apps.get_model('auth', 'Permission') # noqa ThroughModel = User.user_permissions.through # noqa - permission = Permission.objects.get( - content_type__app_label='logger', codename='delete_data_xform' - ) - user_ids = User.objects.values_list('pk', flat=True).exclude( - pk=AnonymousUser().pk - ) - - through_models = [] - for user_id in user_ids: - through_models.append( - ThroughModel(user_id=user_id, permission_id=permission.pk) + with use_db(settings.OPENROSA_DB_ALIAS): + permission = Permission.objects.get( + content_type__app_label='logger', codename='delete_data_xform' + ) + user_ids = User.objects.values_list('pk', flat=True).exclude( + pk=AnonymousUser().pk ) - sys.stderr.write( - 'Creating {} model-level permission assignments...\n'.format( - len(through_models) + through_models = [] + for user_id in user_ids: + through_models.append( + ThroughModel(user_id=user_id, permission_id=permission.pk) + ) + + sys.stderr.write( + 'Creating {} model-level permission assignments...\n'.format( + len(through_models) + ) ) - ) - sys.stderr.flush() - # Django 1.8 does not support `ignore_conflicts=True` - ThroughModel.objects.bulk_create(through_models) + sys.stderr.flush() + # Django 1.8 does not support `ignore_conflicts=True` + ThroughModel.objects.bulk_create(through_models) def remove_model_level_perms(apps): @@ -57,12 +63,13 @@ def remove_model_level_perms(apps): Permission = apps.get_model('auth', 'Permission') # noqa ThroughModel = User.user_permissions.through # noqa - permission = Permission.objects.get( - content_type__app_label='logger', codename='delete_data_xform' - ) - ThroughModel.objects.filter(permission=permission).exclude( - user_id=AnonymousUser().pk - ).delete() + with use_db(settings.OPENROSA_DB_ALIAS): + permission = Permission.objects.get( + content_type__app_label='logger', codename='delete_data_xform' + ) + ThroughModel.objects.filter(permission=permission).exclude( + user_id=AnonymousUser().pk + ).delete() def grant_object_level_perms(apps): @@ -72,31 +79,30 @@ def grant_object_level_perms(apps): """ User = apps.get_model('kobo_auth', 'User') # noqa Permission = apps.get_model('auth', 'Permission') # noqa - UserObjectPermission = apps.get_model( - 'guardian', 'UserObjectPermission' - ) # noqa + UserObjectPermission = apps.get_model('guardian', 'UserObjectPermission') # noqa - new_perm = Permission.objects.get( - content_type__app_label='logger', codename='delete_data_xform' - ) - old_perm = Permission.objects.get( - content_type__app_label='logger', codename='change_xform' - ) - new_perm_objects = [] - for old_assign in UserObjectPermission.objects.filter( - permission=old_perm - ).iterator(): - old_assign.pk = None - old_assign.permission = new_perm - new_perm_objects.append(old_assign) - sys.stderr.write( - 'Creating {} object-level permission assignments...\n'.format( - len(new_perm_objects) + with use_db(settings.OPENROSA_DB_ALIAS): + new_perm = Permission.objects.get( + content_type__app_label='logger', codename='delete_data_xform' ) - ) - sys.stderr.flush() - # Django 1.8 does not support `ignore_conflicts=True` - UserObjectPermission.objects.bulk_create(new_perm_objects) + old_perm = Permission.objects.get( + content_type__app_label='logger', codename='change_xform' + ) + new_perm_objects = [] + for old_assign in UserObjectPermission.objects.filter( + permission=old_perm + ).iterator(): + old_assign.pk = None + old_assign.permission = new_perm + new_perm_objects.append(old_assign) + sys.stderr.write( + 'Creating {} object-level permission assignments...\n'.format( + len(new_perm_objects) + ) + ) + sys.stderr.flush() + # Django 1.8 does not support `ignore_conflicts=True` + UserObjectPermission.objects.bulk_create(new_perm_objects) def remove_object_level_perms(apps): @@ -104,13 +110,12 @@ def remove_object_level_perms(apps): Remove all object-level 'delete_submissions' permission assignments """ Permission = apps.get_model('auth', 'Permission') # noqa - UserObjectPermission = apps.get_model( - 'guardian', 'UserObjectPermission' - ) # noqa - perm = Permission.objects.get( - content_type__app_label='logger', codename='delete_data_xform' - ) - UserObjectPermission.objects.filter(permission=perm).delete() + UserObjectPermission = apps.get_model('guardian', 'UserObjectPermission') # noqa + with use_db(settings.OPENROSA_DB_ALIAS): + perm = Permission.objects.get( + content_type__app_label='logger', codename='delete_data_xform' + ) + UserObjectPermission.objects.filter(permission=perm).delete() def forwards_func(apps, schema_editor): diff --git a/kobo/apps/openrosa/apps/main/migrations/0011_drop_old_kpi_tables.py b/kobo/apps/openrosa/apps/main/migrations/0011_drop_old_kpi_tables.py index ba87f751ee..939f09a271 100644 --- a/kobo/apps/openrosa/apps/main/migrations/0011_drop_old_kpi_tables.py +++ b/kobo/apps/openrosa/apps/main/migrations/0011_drop_old_kpi_tables.py @@ -1,6 +1,6 @@ # Generated by Django 3.2.15 on 2023-03-22 14:05 -from django.db import migrations, connection +from django.db import migrations, connections from django.conf import settings @@ -70,7 +70,7 @@ def get_operations(): WHERE nsp.nspname = 'public' AND rel.relname = %s; """ - with connection.cursor() as cursor: + with connections[settings.OPENROSA_DB_ALIAS].cursor() as cursor: drop_table_queries = [] for table in tables: cursor.execute(sql, [table]) diff --git a/kobo/apps/openrosa/apps/main/models/user_profile.py b/kobo/apps/openrosa/apps/main/models/user_profile.py index ac4305b610..3f407c264c 100644 --- a/kobo/apps/openrosa/apps/main/models/user_profile.py +++ b/kobo/apps/openrosa/apps/main/models/user_profile.py @@ -18,6 +18,7 @@ gravatar_exists, ) from kpi.utils.database import use_db +from kpi.utils.permissions import is_user_anonymous class UserProfile(models.Model): @@ -75,6 +76,10 @@ class Meta: # 1) KC Token object is created when KPI calls `KobocatToken.sync()` # in `kpi.signals.save_kobocat_token()` def create_auth_token(sender, instance=None, created=False, **kwargs): + + if is_user_anonymous(instance): + return + if created: with use_db(settings.OPENROSA_DB_ALIAS): if User.objects.filter(pk=instance.pk).exists() or settings.TESTING: diff --git a/kobo/apps/openrosa/apps/main/signals.py b/kobo/apps/openrosa/apps/main/signals.py index a498fe35a8..6dee3797bc 100644 --- a/kobo/apps/openrosa/apps/main/signals.py +++ b/kobo/apps/openrosa/apps/main/signals.py @@ -2,6 +2,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.utils.database import use_db +from kpi.utils.permissions import is_user_anonymous def set_api_permissions(sender, instance=None, created=False, **kwargs): @@ -9,6 +10,10 @@ def set_api_permissions(sender, instance=None, created=False, **kwargs): # KC object permissions when its calling `grant_kc_model_level_perms()` # in `kpi.signals.save_kobocat_user()` from kobo.apps.openrosa.libs.utils.user_auth import set_api_permissions_for_user + + if is_user_anonymous(instance): + return + if created: with use_db(settings.OPENROSA_DB_ALIAS): if User.objects.filter(pk=instance.pk).exists() or settings.TESTING: diff --git a/kobo/apps/openrosa/libs/data/query.py b/kobo/apps/openrosa/libs/data/query.py index c57b734130..45738f6c3a 100644 --- a/kobo/apps/openrosa/libs/data/query.py +++ b/kobo/apps/openrosa/libs/data/query.py @@ -1,6 +1,6 @@ # coding: utf-8 from django.conf import settings -from django.db import connection +from django.db import connections from kobo.apps.openrosa.libs.utils.common_tags import SUBMISSION_TIME @@ -27,7 +27,7 @@ def _dictfetchall(cursor): def _execute_query(query, to_dict=True): - cursor = connection.cursor() + cursor = connections[settings.OPENROSA_DB_ALIAS].cursor() cursor.execute(query) return _dictfetchall(cursor) if to_dict else cursor diff --git a/kobo/settings/guardian.py b/kobo/settings/guardian.py new file mode 100644 index 0000000000..49f381cacb --- /dev/null +++ b/kobo/settings/guardian.py @@ -0,0 +1,8 @@ +# coding: utf-8 +from .prod import * + +# This file is only used to run migrations without the creation of the +# (django-guardian) AnonymousUser to avoid race condition while running +# migrations for the first time. +# e.g.: DJANGO_SETTINGS_MODULE=kobo.settings.guardian python manage.py migrate +ANONYMOUS_USER_NAME = None diff --git a/kpi/constants.py b/kpi/constants.py index 7498196bfd..d4bc8ad9f5 100644 --- a/kpi/constants.py +++ b/kpi/constants.py @@ -72,6 +72,16 @@ 'superuser_stats', ] +SHARED_APP_LABELS = [ + 'auth', + 'contenttypes', + 'django_digest', + 'kobo_auth', + 'reversion', + 'sessions', +] + + # List of nested attributes which bypass 'dots' encoding NESTED_MONGO_RESERVED_ATTRIBUTES = [ "_validation_status", diff --git a/kpi/db_routers.py b/kpi/db_routers.py index de076dd127..75840d5e2e 100644 --- a/kpi/db_routers.py +++ b/kpi/db_routers.py @@ -5,7 +5,7 @@ OPENROSA_APP_LABELS, ) from kpi.utils.database import get_thread_local -from .constants import SHADOW_MODEL_APP_LABELS +from .constants import SHADOW_MODEL_APP_LABELS, SHARED_APP_LABELS from .exceptions import ReadOnlyModelError @@ -52,6 +52,9 @@ def allow_migrate(self, db, app_label, model=None, **hints): """ All default models end up in this pool. """ + if app_label in SHARED_APP_LABELS: + return True + if db == DEFAULT_DB_ALIAS and app_label in OPENROSA_APP_LABELS: return False diff --git a/kpi/deployment_backends/kc_access/shadow_models.py b/kpi/deployment_backends/kc_access/shadow_models.py index d049437ad6..0c22815175 100644 --- a/kpi/deployment_backends/kc_access/shadow_models.py +++ b/kpi/deployment_backends/kc_access/shadow_models.py @@ -37,9 +37,18 @@ def update_autofield_sequence(model): Fixes the PostgreSQL sequence for the first (and only?) `AutoField` on `model`, à la `manage.py sqlsequencereset` """ + # Updating sequences on fresh environments fails because the only user + # in the DB is django-guardian AnonymousUser and `max(pk)` returns -1. + # Error: + # > setval: value -1 is out of bounds for sequence + # Using abs() and testing if max(pk) equals -1, leaves the sequence alone. sql_template = ( - "SELECT setval(pg_get_serial_sequence('{table}','{column}'), " - "coalesce(max({column}), 1), max({column}) IS NOT null) FROM {table};" + "SELECT setval(" + " pg_get_serial_sequence('{table}','{column}'), " + " abs(coalesce(max({column}), 1)), " + " max({column}) IS NOT null and max({column}) != -1" + ") " + "FROM {table};" ) autofield = None for f in model._meta.get_fields(): @@ -455,7 +464,7 @@ class Meta(ShadowModel.Meta): @transaction.atomic def sync(cls, auth_user): # NB: `KobocatUserObjectPermission` (and probably other things) depend - # upon PKs being synchronized between KPI and KoBoCAT + # upon PKs being synchronized between KPI and KoboCAT kc_auth_user = cls.get_kc_user(auth_user) kc_auth_user.password = auth_user.password kc_auth_user.last_login = auth_user.last_login @@ -473,7 +482,7 @@ def sync(cls, auth_user): # `auth_user_id_seq` now lags behind `max(id)`. Fix it now! update_autofield_sequence(cls) - # Update django-digest `PartialDigest`s in KoBoCAT. This is only + # Update django-digest `PartialDigest`s in KoboCAT. This is only # necessary if the user's password has changed, but we do it always KobocatDigestPartial.sync(kc_auth_user) diff --git a/kpi/migrations/0055_set_require_auth_per_project.py b/kpi/migrations/0055_set_require_auth_per_project.py index 1bfd59ded7..26a1a3c335 100644 --- a/kpi/migrations/0055_set_require_auth_per_project.py +++ b/kpi/migrations/0055_set_require_auth_per_project.py @@ -3,9 +3,11 @@ from django.conf import settings from django.db import migrations +from django.db.utils import ProgrammingError +from kobo.apps.openrosa.apps.main.models import UserProfile from kpi.constants import PERM_ADD_SUBMISSIONS, SKIP_HEAVY_MIGRATIONS_GUIDANCE -from kpi.deployment_backends.kc_access.shadow_models import KobocatUserProfile + CHUNK_SIZE = 2000 @@ -21,8 +23,15 @@ def assign_add_submissions_to_anonymous_users(apps, schema_editor): Permission = apps.get_model('auth', 'Permission') # noqa permission_id = Permission.objects.get(codename=PERM_ADD_SUBMISSIONS).pk + try: + UserProfile.objects.first() + except ProgrammingError as e: + # Race condition when KoboCAT migrations have not run yet + if 'relation "main_userprofile" does not exist' in str(e): + return + owner_iter = ( - KobocatUserProfile.objects.filter(require_auth=False) + UserProfile.objects.filter(require_auth=False) .values_list('user_id', flat=True) .iterator(chunk_size=CHUNK_SIZE) ) diff --git a/kpi/signals.py b/kpi/signals.py index ad43039b7d..5d5a371453 100644 --- a/kpi/signals.py +++ b/kpi/signals.py @@ -49,9 +49,10 @@ def default_permissions_post_save(sender, instance, created, raw, **kwargs): def save_kobocat_user(sender, instance, created, raw, **kwargs): """ Sync auth_user table between KPI and KC, and, if the user is newly created, - grant all KoBoCAT model-level permissions for the content types listed in + grant all KoboCAT model-level permissions for the content types listed in `settings.KOBOCAT_DEFAULT_PERMISSION_CONTENT_TYPES` """ + if not settings.TESTING: with kc_transaction_atomic(): KobocatUser.sync(instance) diff --git a/scripts/create_anonymous_user.py b/scripts/create_anonymous_user.py new file mode 100644 index 0000000000..32e6326454 --- /dev/null +++ b/scripts/create_anonymous_user.py @@ -0,0 +1,9 @@ +from kpi.utils.object_permission import get_anonymous_user + + +def run(): + """ + Simple call to `get_anonymous_user()` to create the AnonymousUser if + it doesn't exist. + """ + get_anonymous_user() diff --git a/scripts/fix_migrations_for_kobocat_django_app.py b/scripts/fix_migrations_for_kobocat_django_app.py index 5cbd32395d..f8350d18cc 100644 --- a/scripts/fix_migrations_for_kobocat_django_app.py +++ b/scripts/fix_migrations_for_kobocat_django_app.py @@ -3,11 +3,27 @@ def run(): + if not are_migration_already_applied(): + return + if migrate_custom_user_model(): # Only run it when custom user model migrations have been fixed delete_kobocat_form_disclaimer_app() +def are_migration_already_applied(): + with connection.cursor() as cursor: + cursor.execute( + "SELECT EXISTS (" + " SELECT FROM pg_tables" + " WHERE schemaname = 'public'" + " AND tablename = 'django_migrations'" + ");" + ) + row = cursor.fetchone() + return bool(row[0]) + + def delete_kobocat_form_disclaimer_app(): """ KoboCAT form_disclaimer app does not exist anymore but its migrations diff --git a/scripts/migrate.sh b/scripts/migrate.sh index 79d6c679ff..b9e5ad3297 100755 --- a/scripts/migrate.sh +++ b/scripts/migrate.sh @@ -2,5 +2,8 @@ set -e python manage.py runscript fix_migrations_for_kobocat_django_app -python manage.py migrate --noinput -python manage.py migrate --noinput --database kobocat +echo '########## KPI migrations ############' +DJANGO_SETTINGS_MODULE=kobo.settings.guardian python manage.py migrate --noinput +echo '########## KoboCAT migrations ############' +DJANGO_SETTINGS_MODULE=kobo.settings.guardian python manage.py migrate --noinput --database kobocat +python manage.py runscript create_anonymous_user From 12562b1aa3fef8e603787340bd1ba4ad48c38bc5 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 17 Jun 2024 15:59:30 -0400 Subject: [PATCH 19/21] Add taggit to shared django apps --- kpi/constants.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kpi/constants.py b/kpi/constants.py index d4bc8ad9f5..2e5382a717 100644 --- a/kpi/constants.py +++ b/kpi/constants.py @@ -79,6 +79,7 @@ 'kobo_auth', 'reversion', 'sessions', + 'taggit', ] From cd66c3aa417b021e5db3fe246a6d8e0641b43519 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 17 Jun 2024 16:00:05 -0400 Subject: [PATCH 20/21] Better detection of database from permission codename --- kobo/apps/openrosa/libs/permissions.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kobo/apps/openrosa/libs/permissions.py b/kobo/apps/openrosa/libs/permissions.py index f60304203d..5ce7c4ce3f 100644 --- a/kobo/apps/openrosa/libs/permissions.py +++ b/kobo/apps/openrosa/libs/permissions.py @@ -28,6 +28,8 @@ def get_object_users_with_permissions(obj, exclude=None, serializable=False): @cache_for_request def get_model_permission_codenames(): - return Permission.objects.using(settings.OPENROSA_DB_ALIAS).values_list( + kc_perms = set(Permission.objects.using(settings.OPENROSA_DB_ALIAS).values_list( 'codename', flat=True - ) + )) + kpi_perms = set(Permission.objects.values_list('codename', flat=True)) + return list(kc_perms - kpi_perms) From f6173667b6fbd7de9e0759e49690d6bf7ffc9677 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 18 Jun 2024 15:14:56 -0400 Subject: [PATCH 21/21] Create user's profile when validation password from admin --- hub/admin/password_validation.py | 29 +++++++++++++++---- .../commands/create_kobo_superuser.py | 3 ++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/hub/admin/password_validation.py b/hub/admin/password_validation.py index b7d3a95752..bf8e67e15d 100644 --- a/hub/admin/password_validation.py +++ b/hub/admin/password_validation.py @@ -3,7 +3,7 @@ from django.db import transaction from django.utils.html import format_html -from kpi.deployment_backends.kc_access.shadow_models import KobocatUserProfile +from kobo.apps.openrosa.apps.main.models import UserProfile from .filters import PasswordValidationAdvancedSearchFilter from .mixins import AdvancedSearchMixin from ..models import ExtraUserDetail @@ -67,12 +67,17 @@ def get_queryset(self, request): @admin.display(description='Validated') def get_validated_password(self, obj): - value = True + value = False try: value = obj.extra_details.validated_password except obj.extra_details.RelatedObjectDoesNotExist: pass + try: + value = value and obj.profile.validated_password + except obj.profile.RelatedObjectDoesNotExist: + pass + return format_html( '{}', 'yes' if value else 'no', @@ -88,8 +93,14 @@ def invalidate_passwords(self, request, queryset, **kwargs): ExtraUserDetail.objects.filter(user_id__in=user_ids).update( validated_password=False ) - KobocatUserProfile.objects.filter(user_id__in=user_ids).update( - validated_password=False + UserProfile.objects.bulk_create( + [ + UserProfile(user_id=user_id, validated_password=False) + for user_id in user_ids + ], + update_conflicts=True, + unique_fields=['user_id'], + update_fields=['validated_password'], ) self.message_user( @@ -107,8 +118,14 @@ def validate_passwords(self, request, queryset, **kwargs): ExtraUserDetail.objects.filter(user_id__in=user_ids).update( validated_password=True ) - KobocatUserProfile.objects.filter(user_id__in=user_ids).update( - validated_password=True + UserProfile.objects.bulk_create( + [ + UserProfile(user_id=user_id, validated_password=True) + for user_id in user_ids + ], + update_conflicts=True, + unique_fields=['user_id'], + update_fields=['validated_password'], ) self.message_user( diff --git a/kpi/management/commands/create_kobo_superuser.py b/kpi/management/commands/create_kobo_superuser.py index 53ea2bf4eb..f2329b6a08 100644 --- a/kpi/management/commands/create_kobo_superuser.py +++ b/kpi/management/commands/create_kobo_superuser.py @@ -6,6 +6,7 @@ from django.db.utils import ProgrammingError from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.openrosa.apps.main.models import UserProfile class Command(BaseCommand): @@ -29,6 +30,8 @@ def handle(self, *args, **options): except Exception as e: self.stdout.write('Superuser could not be created.\n' 'Error: {}'.format(str(e))) + else: + UserProfile.objects.create(user=user) if User.objects.filter(username=super_username).count() > 0: self.stdout.write('Superuser successfully created.')