From 760298cab2981c6f0e7b450d67277e5a6faa0a71 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 28 Jun 2022 22:13:40 +0530 Subject: [PATCH 01/47] intial_websockets_setup --- contentcuration/contentcuration/asgi.py | 18 +++++ .../frontend/shared/data/serverSync.js | 38 ++++++++--- contentcuration/contentcuration/settings.py | 11 ++++ .../tests/test_websocket_consumer.py | 44 +++++++++++++ .../viewsets/websockets/consumers.py | 65 +++++++++++++++++++ .../viewsets/websockets/routing.py | 7 ++ docker-compose.yml | 2 +- 7 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 contentcuration/contentcuration/asgi.py create mode 100644 contentcuration/contentcuration/tests/test_websocket_consumer.py create mode 100644 contentcuration/contentcuration/viewsets/websockets/consumers.py create mode 100644 contentcuration/contentcuration/viewsets/websockets/routing.py diff --git a/contentcuration/contentcuration/asgi.py b/contentcuration/contentcuration/asgi.py new file mode 100644 index 0000000000..75b1168d71 --- /dev/null +++ b/contentcuration/contentcuration/asgi.py @@ -0,0 +1,18 @@ +import os + +from channels.auth import AuthMiddlewareStack +from channels.routing import ProtocolTypeRouter +from channels.routing import URLRouter + +from contentcuration.viewsets.websockets.routing import websocket_urlpatterns + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "contentcuration.settings") + +application = ProtocolTypeRouter({ + "websocket": + AuthMiddlewareStack( + URLRouter( + websocket_urlpatterns + ) + ), +}) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 6ffd155898..0d7d4d0ad9 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -1,24 +1,25 @@ import debounce from 'lodash/debounce'; import findLastIndex from 'lodash/findLastIndex'; import get from 'lodash/get'; -import pick from 'lodash/pick'; import orderBy from 'lodash/orderBy'; +import pick from 'lodash/pick'; import uniq from 'lodash/uniq'; -import applyChanges from './applyRemoteChanges'; -import { hasActiveLocks, cleanupLocks } from './changes'; import { + ACTIVE_CHANNELS, + CHANGES_TABLE, CHANGE_LOCKS_TABLE, CHANGE_TYPES, - CHANGES_TABLE, - IGNORED_SOURCE, CHANNEL_SYNC_KEEP_ALIVE_INTERVAL, - ACTIVE_CHANNELS, + IGNORED_SOURCE, MAX_REV_KEY, } from './constants'; +import { Channel, Session, Task } from './resources'; +import { cleanupLocks, hasActiveLocks } from './changes'; + +import { INDEXEDDB_RESOURCES } from './registry'; +import applyChanges from './applyRemoteChanges'; import db from './db'; import mergeAllChanges from './mergeChanges'; -import { INDEXEDDB_RESOURCES } from './registry'; -import { Channel, Session, Task } from './resources'; import client from 'shared/client'; import urls from 'shared/urls'; @@ -336,12 +337,33 @@ async function handleChanges(changes) { } } +function initializeWebsockets() { + // Create WebSocket connection. + const socket = new WebSocket('ws://localhost:8080/ws/sync_socket/12312312312123/'); + socket.onopen = function() { + console.log('WebSocket connection opened.'); + console.log(window.CHANNEL_EDIT_GLOBAL['channel_id']); + }; + + // Connection opened + socket.addEventListener('open', function(event) { + socket.send('Hello Server!'); + console.log(event); + }); + + // Listen for messages + socket.addEventListener('message', function(event) { + console.log('Message from server ', event.data); + }); +} + let intervalTimer; export function startSyncing() { cleanupLocks(); // Initiate a sync immediately in case any data // is left over in the database. + initializeWebsockets(); debouncedSyncChanges(); // Start the sync interval intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); diff --git a/contentcuration/contentcuration/settings.py b/contentcuration/contentcuration/settings.py index ae637542ba..80678aac2c 100644 --- a/contentcuration/contentcuration/settings.py +++ b/contentcuration/contentcuration/settings.py @@ -86,6 +86,7 @@ 'webpack_loader', 'django_filters', 'mathfilters', + 'channels', ) SESSION_ENGINE = "django.contrib.sessions.backends.cached_db" @@ -201,8 +202,18 @@ ] WSGI_APPLICATION = 'contentcuration.wsgi.application' +ASGI_APPLICATION = 'contentcuration.asgi.application' +CHANNEL_LAYERS = { + 'default': { + 'BACKEND': 'channels_redis.core.RedisChannelLayer', + 'CONFIG': { + "hosts": [('127.0.0.1', 6379)], + }, + }, +} + # Database # https://docs.djangoproject.com/en/1.8/ref/settings/#databases diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py new file mode 100644 index 0000000000..471b217898 --- /dev/null +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -0,0 +1,44 @@ +import os + +import pytest +from channels.auth import AuthMiddlewareStack +from channels.db import database_sync_to_async +from channels.routing import URLRouter +from channels.testing import WebsocketCommunicator +from django.test import TestCase +from django.urls import re_path + +from ..viewsets.websockets.consumers import SyncConsumer +from contentcuration.models import User +# from .base import BaseAPITestCase + +application = AuthMiddlewareStack( + URLRouter([ + re_path(r'ws/sync_socket/(?P\w+)/$', SyncConsumer.as_asgi()), + ]) +) + +os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" + + +@database_sync_to_async +def create_user(self): + user = User.objects.create( + email="mrtest@testy.com", + ) + user.set_password("password") + user.save() + return user + + +class WebsocketTestCase(TestCase): + @pytest.mark.asyncio + async def test_websocket_connection(self): + user = create_user() + self.client.force_authenticate(user) + headers = [(b'origin', b'...'), (b'cookie', self.client.cookies.output(header='', sep='; ').encode())] + communicator = WebsocketCommunicator(application, '/ws/sync_socket/12312312312123/', headers) + connected, subprotocol = await communicator.connect() + assert connected + # Close + await communicator.disconnect() diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py new file mode 100644 index 0000000000..74219104c0 --- /dev/null +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -0,0 +1,65 @@ +from asgiref.sync import async_to_sync +from channels.generic.websocket import WebsocketConsumer + + +class SyncConsumer(WebsocketConsumer): + + # Checks permissions + def check_perms(self): + return self.scope['user'].is_authenticated + + # Initial reset + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.room_name = None + self.room_group_name = None + self.user_id = None + + # This function gets executed when a user tries to make a websocket connection. + # - Creates and joins a group for indiviual user + # - Joins a public group based on channel_id provided in url + def connect(self): + + # Extract the channel_id from url + self.room_name = self.scope['url_route']['kwargs']['channel_id'] + self.room_group_name = self.room_name + print("----------------------------------") + print("Connected to channel_id: " + self.room_name) + print("----------------------------------") + # Create a private group for + self.user = self.scope["user"] + self.indiviual_room_group_name = "asdasdasdasd" + print("----------------------------------") + print("Connected to user_id: " + str(self.user)) + print("----------------------------------") + + if self.scope['user'].is_authenticated: + # Join room group based on channel_id + async_to_sync(self.channel_layer.group_add)( + self.room_group_name, + self.channel_name + ) + + # Join private room group for indiviual user + async_to_sync(self.channel_layer.group_add)( + self.indiviual_room_group_name, + self.channel_name + ) + + self.accept() + + else: + self.close() + + def disconnect(self, close_code): + # Leave channel_id room group + async_to_sync(self.channel_layer.group_discard)( + self.room_group_name, + self.channel_name + ) + + # Leave indiviual room group + async_to_sync(self.channel_layer.group_discard)( + self.indiviual_room_group_name, + self.channel_name + ) diff --git a/contentcuration/contentcuration/viewsets/websockets/routing.py b/contentcuration/contentcuration/viewsets/websockets/routing.py new file mode 100644 index 0000000000..c334df7b03 --- /dev/null +++ b/contentcuration/contentcuration/viewsets/websockets/routing.py @@ -0,0 +1,7 @@ +from django.urls import re_path + +from . import consumers + +websocket_urlpatterns = [ + re_path(r'ws/sync_socket/(?P\w+)/$', consumers.SyncConsumer.as_asgi()), +] diff --git a/docker-compose.yml b/docker-compose.yml index 02732ba585..f6b51ddc9d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -63,7 +63,7 @@ services: - pgdata:/var/lib/postgresql/data/pgdata redis: - image: redis:4.0.9 + image: redis:6.0.9 cloudprober: <<: *studio-worker From 798822e2b5072cd87021d98e835cab4340a3c5ad Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 28 Jun 2022 22:54:43 +0530 Subject: [PATCH 02/47] try_fixin_INSTALLED_APPS_error --- contentcuration/contentcuration/settings.py | 3 ++- .../contentcuration/tests/test_websocket_consumer.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/settings.py b/contentcuration/contentcuration/settings.py index 80678aac2c..24a091d6b8 100644 --- a/contentcuration/contentcuration/settings.py +++ b/contentcuration/contentcuration/settings.py @@ -67,6 +67,7 @@ # Application definition INSTALLED_APPS = ( + 'channels', 'contentcuration.apps.ContentConfig', 'django.contrib.auth', 'django.contrib.contenttypes', @@ -86,7 +87,7 @@ 'webpack_loader', 'django_filters', 'mathfilters', - 'channels', + ) SESSION_ENGINE = "django.contrib.sessions.backends.cached_db" diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index 471b217898..dd43082a61 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -34,8 +34,8 @@ def create_user(self): class WebsocketTestCase(TestCase): @pytest.mark.asyncio async def test_websocket_connection(self): - user = create_user() - self.client.force_authenticate(user) + create_user() + self.client.force_login(username="mrtest@testy.com", password="password") headers = [(b'origin', b'...'), (b'cookie', self.client.cookies.output(header='', sep='; ').encode())] communicator = WebsocketCommunicator(application, '/ws/sync_socket/12312312312123/', headers) connected, subprotocol = await communicator.connect() From 9d2f27ce695cdb8b63b3247eca81b3cf083931bd Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 28 Jun 2022 23:08:47 +0530 Subject: [PATCH 03/47] updated requirements.in file --- requirements.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/requirements.in b/requirements.in index 254eb376fb..22dad3360e 100644 --- a/requirements.in +++ b/requirements.in @@ -44,3 +44,5 @@ pillow==8.3.2 python-dateutil>=2.8.1 jsonschema>=3.2.0 importlib-metadata==1.7.0 +channels==3.0.4 +channels-redis==3.3.1 From a3bc91b93927baf4c51c0603ce887b77a8c18e4c Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 28 Jun 2022 23:21:25 +0530 Subject: [PATCH 04/47] FIX_Django_Settings_Error --- .../tests/test_websocket_consumer.py | 4 +- requirements.txt | 122 ++++++++++++++---- 2 files changed, 97 insertions(+), 29 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index dd43082a61..6f18df4929 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -34,8 +34,8 @@ def create_user(self): class WebsocketTestCase(TestCase): @pytest.mark.asyncio async def test_websocket_connection(self): - create_user() - self.client.force_login(username="mrtest@testy.com", password="password") + user = create_user() + self.client.force_login(user) headers = [(b'origin', b'...'), (b'cookie', self.client.cookies.output(header='', sep='; ').encode())] communicator = WebsocketCommunicator(application, '/ws/sync_socket/12312312312123/', headers) connected, subprotocol = await communicator.connect() diff --git a/requirements.txt b/requirements.txt index aa81be7159..8ad2b75e5f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,14 +4,29 @@ # # pip-compile requirements.in # +aioredis==1.3.1 + # via channels-redis amqp==2.6.1 # via kombu asgiref==3.3.4 - # via django + # via + # channels + # channels-redis + # daphne + # django +async-timeout==4.0.2 + # via aioredis attrs==19.3.0 # via # -r requirements.in + # automat # jsonschema + # service-identity + # twisted +autobahn==21.2.1 + # via daphne +automat==20.2.0 + # via twisted backoff==1.10.0 # via -r requirements.in backports-abc==0.5 @@ -33,25 +48,28 @@ certifi==2020.12.5 # requests # sentry-sdk cffi==1.14.5 - # via google-crc32c + # via + # cryptography + # google-crc32c +channels-redis==3.3.1 + # via -r requirements.in +channels==3.0.4 + # via + # -r requirements.in + # channels-redis chardet==4.0.0 # via requests confusable-homoglyphs==3.2.0 # via django-registration -django==3.2.13 +constantly==15.1.0 + # via twisted +cryptography==37.0.2 # via - # -r requirements.in - # django-bulk-update - # django-db-readonly - # django-filter - # django-js-reverse - # django-model-utils - # django-mptt - # django-redis - # django-registration - # django-s3-storage - # djangorestframework - # jsonfield + # autobahn + # pyopenssl + # service-identity +daphne==3.0.2 + # via channels django-bulk-update==2.2.0 # via -r requirements.in django-cte==1.1.5 @@ -82,6 +100,21 @@ django-s3-storage==0.13.4 # via -r requirements.in django-webpack-loader==0.7.0 # via -r requirements.in +django==3.2.13 + # via + # -r requirements.in + # channels + # django-bulk-update + # django-db-readonly + # django-filter + # django-js-reverse + # django-model-utils + # django-mptt + # django-redis + # django-registration + # django-s3-storage + # djangorestframework + # jsonfield djangorestframework==3.12.4 # via -r requirements.in future==0.18.2 @@ -95,6 +128,10 @@ google-api-core[grpc]==1.27.0 # google-cloud-logging google-api-python-client==2.4.0 # via -r requirements.in +google-auth-httplib2==0.1.0 + # via google-api-python-client +google-auth-oauthlib==0.4.4 + # via gspread google-auth==1.30.0 # via # google-api-core @@ -104,10 +141,6 @@ google-auth==1.30.0 # google-cloud-core # google-cloud-storage # gspread -google-auth-httplib2==0.1.0 - # via google-api-python-client -google-auth-oauthlib==0.4.4 - # via gspread google-cloud-core==1.6.0 # via # -r requirements.in @@ -140,6 +173,8 @@ gspread==3.6.0 # via -r requirements.in gunicorn==19.6.0 # via -r requirements.in +hiredis==2.0.0 + # via aioredis html5lib==1.1 # via -r requirements.in httplib2==0.19.1 @@ -148,10 +183,22 @@ httplib2==0.19.1 # google-api-python-client # google-auth-httplib2 # oauth2client +hyperlink==21.0.0 + # via + # autobahn + # twisted idna==2.10 - # via requests + # via + # hyperlink + # requests + # twisted importlib-metadata==1.7.0 - # via -r requirements.in + # via + # -r requirements.in + # jsonschema + # kombu +incremental==21.3.0 + # via twisted jmespath==0.10.0 # via # boto3 @@ -164,6 +211,8 @@ kombu==4.6.11 # via celery le-utils==0.1.40 # via -r requirements.in +msgpack==1.0.4 + # via channels-redis newrelic==6.2.0.156 # via -r requirements.in oauth2client==4.1.3 @@ -191,19 +240,23 @@ protobuf==3.17.0 # proto-plus psycopg2-binary==2.8.6 # via -r requirements.in +pyasn1-modules==0.2.8 + # via + # google-auth + # oauth2client + # service-identity pyasn1==0.4.8 # via # oauth2client # pyasn1-modules # rsa -pyasn1-modules==0.2.8 - # via - # google-auth - # oauth2client + # service-identity pycountry==17.5.14 # via -r requirements.in pycparser==2.20 # via cffi +pyopenssl==22.0.0 + # via twisted pyparsing==2.4.7 # via # httplib2 @@ -230,6 +283,8 @@ redis==3.5.3 # via # -r requirements.in # django-redis +requests-oauthlib==1.3.0 + # via google-auth-oauthlib requests==2.25.1 # via # -r requirements.in @@ -237,8 +292,6 @@ requests==2.25.1 # google-cloud-storage # gspread # requests-oauthlib -requests-oauthlib==1.3.0 - # via google-auth-oauthlib rsa==4.7.2 # via # google-auth @@ -247,8 +300,11 @@ s3transfer==0.4.2 # via boto3 sentry-sdk==1.1.0 # via -r requirements.in +service-identity==21.1.0 + # via twisted six==1.16.0 # via + # automat # google-api-core # google-api-python-client # google-auth @@ -263,8 +319,18 @@ six==1.16.0 # protobuf # python-dateutil # python-utils + # service-identity sqlparse==0.4.1 # via django +twisted[tls]==22.4.0 + # via daphne +txaio==22.2.1 + # via autobahn +typing-extensions==4.1.1 + # via + # asgiref + # async-timeout + # twisted uritemplate==3.0.1 # via google-api-python-client urllib3==1.26.5 @@ -280,6 +346,8 @@ webencodings==0.5.1 # via html5lib zipp==3.4.1 # via importlib-metadata +zope.interface==5.4.0 + # via twisted # The following packages are considered to be unsafe in a requirements file: # setuptools From 44ef33901eac03e6242332ab2efadace06f3c0dc Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 30 Jun 2022 11:55:20 +0530 Subject: [PATCH 05/47] FIX_test_cases --- .../frontend/shared/data/serverSync.js | 2 +- .../tests/test_websocket_consumer.py | 47 ++++++--------- requirements-dev.txt | 43 +++++++++++--- requirements.txt | 58 +++++++++---------- 4 files changed, 81 insertions(+), 69 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 0d7d4d0ad9..1d05e8ef2c 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -339,7 +339,7 @@ async function handleChanges(changes) { function initializeWebsockets() { // Create WebSocket connection. - const socket = new WebSocket('ws://localhost:8080/ws/sync_socket/12312312312123/'); + const socket = new WebSocket('ws://' + window.location.host + '/ws/sync_socket/12312312312123/'); socket.onopen = function() { console.log('WebSocket connection opened.'); console.log(window.CHANNEL_EDIT_GLOBAL['channel_id']); diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index 6f18df4929..c5de7ede1f 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -1,44 +1,31 @@ import os import pytest -from channels.auth import AuthMiddlewareStack -from channels.db import database_sync_to_async -from channels.routing import URLRouter from channels.testing import WebsocketCommunicator -from django.test import TestCase -from django.urls import re_path +from django.core.management import call_command +from django.test import TransactionTestCase -from ..viewsets.websockets.consumers import SyncConsumer -from contentcuration.models import User -# from .base import BaseAPITestCase - -application = AuthMiddlewareStack( - URLRouter([ - re_path(r'ws/sync_socket/(?P\w+)/$', SyncConsumer.as_asgi()), - ]) -) +from contentcuration.asgi import application +from contentcuration.tests import testdata os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" -@database_sync_to_async -def create_user(self): - user = User.objects.create( - email="mrtest@testy.com", - ) - user.set_password("password") - user.save() - return user +class WebsocketTestCase(TransactionTestCase): + serialized_rollback = True + + def setUp(self): + call_command("loadconstants") + self.user = testdata.user("mrtest@testy.com") + def tearDown(self): + self.user.delete() -class WebsocketTestCase(TestCase): @pytest.mark.asyncio - async def test_websocket_connection(self): - user = create_user() - self.client.force_login(user) - headers = [(b'origin', b'...'), (b'cookie', self.client.cookies.output(header='', sep='; ').encode())] - communicator = WebsocketCommunicator(application, '/ws/sync_socket/12312312312123/', headers) - connected, subprotocol = await communicator.connect() + async def test_authenticated_user_websocket_connection(self): + self.client.force_login(self.user) + headers = [(b'cookie', self.client.cookies.output(attrs=["value"], header='', sep='; ').encode())] + communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) + connected = await communicator.connect() assert connected - # Close await communicator.disconnect() diff --git a/requirements-dev.txt b/requirements-dev.txt index 2961f78cec..e3f9dcaa1f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile +# This file is autogenerated by pip-compile with python 3.6 # To update, run: # # pip-compile requirements-dev.in @@ -14,7 +14,7 @@ asgiref==3.3.4 # via # -c requirements.txt # django -aspy-yaml==1.3.0 +aspy.yaml==1.3.0 # via pre-commit astroid==2.5.6 # via pylint @@ -69,6 +69,8 @@ coverage[toml]==5.5 # pytest-cov customizable-django-profiler @ git+https://github.com/someshchaturvedi/customizable-django-profiler.git # via -r requirements-dev.in +dataclasses==0.8 + # via werkzeug distlib==0.3.1 # via virtualenv django==3.2.13 @@ -130,7 +132,16 @@ idna==2.10 importlib-metadata==1.7.0 # via # -c requirements.txt + # kombu + # pep517 + # pluggy # pre-commit + # pytest + # virtualenv +importlib-resources==5.4.0 + # via + # pre-commit + # virtualenv importmagic==0.1.7 # via -r requirements-dev.in inflection==0.5.1 @@ -179,8 +190,10 @@ mock==4.0.3 # via # -r requirements-dev.in # django-concurrent-test-helper -msgpack==1.0.2 - # via locust +msgpack==1.0.4 + # via + # -c requirements.txt + # locust nodeenv==1.6.0 # via # -r requirements-dev.in @@ -272,7 +285,7 @@ pytz==2021.1 # flower pyyaml==5.4.1 # via - # aspy-yaml + # aspy.yaml # pre-commit pyzmq==22.0.3 # via locust @@ -284,8 +297,10 @@ requests==2.25.1 # locust rope==0.19.0 # via -r requirements-dev.in -ruamel-yaml==0.17.4 +ruamel.yaml==0.17.21 # via drf-yasg +ruamel.yaml.clib==0.2.6 + # via ruamel.yaml service-factory==0.1.6 # via -r requirements-dev.in six==1.16.0 @@ -317,6 +332,12 @@ toml==0.10.2 # pytest tornado==6.1 # via flower +typed-ast==1.4.3 + # via astroid +typing-extensions==4.1.1 + # via + # -c requirements.txt + # asgiref ujson==4.0.2 # via # python-jsonrpc-server @@ -358,10 +379,14 @@ zipp==3.4.1 # via # -c requirements.txt # importlib-metadata -zope-event==4.5.0 - # via gevent -zope-interface==5.4.0 + # importlib-resources + # pep517 +zope.event==4.5.0 # via gevent +zope.interface==5.4.0 + # via + # -c requirements.txt + # gevent # The following packages are considered to be unsafe in a requirements file: # pip diff --git a/requirements.txt b/requirements.txt index 8ad2b75e5f..daee063b7e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile +# This file is autogenerated by pip-compile with python 3.6 # To update, run: # # pip-compile requirements.in @@ -51,12 +51,12 @@ cffi==1.14.5 # via # cryptography # google-crc32c -channels-redis==3.3.1 - # via -r requirements.in channels==3.0.4 # via # -r requirements.in # channels-redis +channels-redis==3.3.1 + # via -r requirements.in chardet==4.0.0 # via requests confusable-homoglyphs==3.2.0 @@ -70,6 +70,21 @@ cryptography==37.0.2 # service-identity daphne==3.0.2 # via channels +django==3.2.13 + # via + # -r requirements.in + # channels + # django-bulk-update + # django-db-readonly + # django-filter + # django-js-reverse + # django-model-utils + # django-mptt + # django-redis + # django-registration + # django-s3-storage + # djangorestframework + # jsonfield django-bulk-update==2.2.0 # via -r requirements.in django-cte==1.1.5 @@ -100,21 +115,6 @@ django-s3-storage==0.13.4 # via -r requirements.in django-webpack-loader==0.7.0 # via -r requirements.in -django==3.2.13 - # via - # -r requirements.in - # channels - # django-bulk-update - # django-db-readonly - # django-filter - # django-js-reverse - # django-model-utils - # django-mptt - # django-redis - # django-registration - # django-s3-storage - # djangorestframework - # jsonfield djangorestframework==3.12.4 # via -r requirements.in future==0.18.2 @@ -128,10 +128,6 @@ google-api-core[grpc]==1.27.0 # google-cloud-logging google-api-python-client==2.4.0 # via -r requirements.in -google-auth-httplib2==0.1.0 - # via google-api-python-client -google-auth-oauthlib==0.4.4 - # via gspread google-auth==1.30.0 # via # google-api-core @@ -141,6 +137,10 @@ google-auth==1.30.0 # google-cloud-core # google-cloud-storage # gspread +google-auth-httplib2==0.1.0 + # via google-api-python-client +google-auth-oauthlib==0.4.4 + # via gspread google-cloud-core==1.6.0 # via # -r requirements.in @@ -240,17 +240,17 @@ protobuf==3.17.0 # proto-plus psycopg2-binary==2.8.6 # via -r requirements.in -pyasn1-modules==0.2.8 - # via - # google-auth - # oauth2client - # service-identity pyasn1==0.4.8 # via # oauth2client # pyasn1-modules # rsa # service-identity +pyasn1-modules==0.2.8 + # via + # google-auth + # oauth2client + # service-identity pycountry==17.5.14 # via -r requirements.in pycparser==2.20 @@ -283,8 +283,6 @@ redis==3.5.3 # via # -r requirements.in # django-redis -requests-oauthlib==1.3.0 - # via google-auth-oauthlib requests==2.25.1 # via # -r requirements.in @@ -292,6 +290,8 @@ requests==2.25.1 # google-cloud-storage # gspread # requests-oauthlib +requests-oauthlib==1.3.0 + # via google-auth-oauthlib rsa==4.7.2 # via # google-auth From f8d02ed7504859a619f8efe26d27c238d6a64b42 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 1 Jul 2022 16:23:38 +0530 Subject: [PATCH 06/47] send_data_from_frontend --- .../frontend/shared/data/serverSync.js | 31 ++++++------------- .../viewsets/websockets/consumers.py | 17 ++++++++++ 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 1d05e8ef2c..60087184e2 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -30,7 +30,7 @@ const SYNC_IF_NO_CHANGES_FOR = 2; // When this many seconds pass, repoll the backend to // check for any updates to active channels, or the user and sync any current changes const SYNC_POLL_INTERVAL = 5; - +let socket; // Flag to check if a sync is currently active. let syncActive = false; @@ -275,6 +275,13 @@ async function syncChanges() { // "errors": [], // "successes": [], // } + if (requestPayload.changes.length != 0) { + socket.send( + JSON.stringify({ + payload: requestPayload, + }) + ); + } const response = await client.post(urls['sync'](), requestPayload); try { await Promise.all([ @@ -337,33 +344,13 @@ async function handleChanges(changes) { } } -function initializeWebsockets() { - // Create WebSocket connection. - const socket = new WebSocket('ws://' + window.location.host + '/ws/sync_socket/12312312312123/'); - socket.onopen = function() { - console.log('WebSocket connection opened.'); - console.log(window.CHANNEL_EDIT_GLOBAL['channel_id']); - }; - - // Connection opened - socket.addEventListener('open', function(event) { - socket.send('Hello Server!'); - console.log(event); - }); - - // Listen for messages - socket.addEventListener('message', function(event) { - console.log('Message from server ', event.data); - }); -} - let intervalTimer; export function startSyncing() { cleanupLocks(); // Initiate a sync immediately in case any data // is left over in the database. - initializeWebsockets(); + socket = new WebSocket('ws://' + window.location.host + '/ws/sync_socket/12312312312123/'); debouncedSyncChanges(); // Start the sync interval intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 74219104c0..d34234527c 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -1,3 +1,5 @@ +import json + from asgiref.sync import async_to_sync from channels.generic.websocket import WebsocketConsumer @@ -63,3 +65,18 @@ def disconnect(self, close_code): self.indiviual_room_group_name, self.channel_name ) + + # Receive message from WebSocket + def receive(self, text_data): + text_data_json = json.loads(text_data) + print(text_data_json) + # message = text_data_json['requestPayload'] + # print(message) + # Send message to room group + # async_to_sync(self.channel_layer.group_send)( + # self.room_group_name, + # { + # 'type': 'chat_message', + # 'message': message + # } + # ) From b0835e3d4506f37c916f3ae6faadd483980d2034 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 1 Jul 2022 17:43:26 +0530 Subject: [PATCH 07/47] inject proper channel_id while making websocket request --- .../frontend/shared/data/serverSync.js | 12 ++++++++++- .../viewsets/websockets/consumers.py | 21 ++++++++----------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 60087184e2..a0d9e7b181 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -350,7 +350,17 @@ export function startSyncing() { cleanupLocks(); // Initiate a sync immediately in case any data // is left over in the database. - socket = new WebSocket('ws://' + window.location.host + '/ws/sync_socket/12312312312123/'); + socket = new WebSocket( + 'ws://' + + window.location.host + + '/ws/sync_socket/' + + window.CHANNEL_EDIT_GLOBAL.channel_id + + '/' + ); + // Connection opened + socket.addEventListener('open', () => { + console.log('Websocket connected'); + }); debouncedSyncChanges(); // Start the sync interval intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index d34234527c..764f65f7e3 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -66,17 +66,14 @@ def disconnect(self, close_code): self.channel_name ) - # Receive message from WebSocket + # Receive message from WebSocket def receive(self, text_data): + session_key = self.scope['cookies']['kolibri_studio_sessionid'] text_data_json = json.loads(text_data) - print(text_data_json) - # message = text_data_json['requestPayload'] - # print(message) - # Send message to room group - # async_to_sync(self.channel_layer.group_send)( - # self.room_group_name, - # { - # 'type': 'chat_message', - # 'message': message - # } - # ) + changes = text_data_json["payload"] + print(session_key) + print(changes) + + # print(session_id) + # When the user sends somme changes then create async tasks and return to him + # {"disallowed": disallowed_changes, "allowed": allowed_changes} From 07041bc3680001dc28b57da54df9f44743ba888f Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Sat, 2 Jul 2022 23:03:55 +0530 Subject: [PATCH 08/47] implement handel_changes from sync api in websockets --- .../frontend/shared/data/serverSync.js | 5 ++ .../viewsets/websockets/consumers.py | 73 ++++++++++++++++--- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index a0d9e7b181..10f2791416 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -283,6 +283,11 @@ async function syncChanges() { ); } const response = await client.post(urls['sync'](), requestPayload); + socket.onmessage = function(e) { + const data = JSON.parse(e.data); + console.log(data); + }; + try { await Promise.all([ handleDisallowed(response), diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 764f65f7e3..1b641dcb13 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -3,11 +3,17 @@ from asgiref.sync import async_to_sync from channels.generic.websocket import WebsocketConsumer +from contentcuration.models import Change +from contentcuration.models import Channel +from contentcuration.tasks import get_or_create_async_task +from contentcuration.viewsets.sync.constants import CHANNEL +from contentcuration.viewsets.sync.constants import CREATED + class SyncConsumer(WebsocketConsumer): # Checks permissions - def check_perms(self): + def check_authentication(self): return self.scope['user'].is_authenticated # Initial reset @@ -15,7 +21,10 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.room_name = None self.room_group_name = None + self.session_key = None + self.user = None self.user_id = None + self.indiviual_room_group_name = None # This function gets executed when a user tries to make a websocket connection. # - Creates and joins a group for indiviual user @@ -25,17 +34,19 @@ def connect(self): # Extract the channel_id from url self.room_name = self.scope['url_route']['kwargs']['channel_id'] self.room_group_name = self.room_name + print("----------------------------------") print("Connected to channel_id: " + self.room_name) print("----------------------------------") - # Create a private group for + self.user = self.scope["user"] - self.indiviual_room_group_name = "asdasdasdasd" + self.indiviual_room_group_name = str(self.user.id) + print("----------------------------------") - print("Connected to user_id: " + str(self.user)) + print("Connected to user " + str(self.user)) print("----------------------------------") - if self.scope['user'].is_authenticated: + if self.check_authentication(): # Join room group based on channel_id async_to_sync(self.channel_layer.group_add)( self.room_group_name, @@ -68,12 +79,50 @@ def disconnect(self, close_code): # Receive message from WebSocket def receive(self, text_data): - session_key = self.scope['cookies']['kolibri_studio_sessionid'] + response_payload = { + "disallowed": [], + "allowed": [], + } + self.user_id = self.scope['user'].id + self.session_key = self.scope['cookies']['kolibri_studio_sessionid'] text_data_json = json.loads(text_data) - changes = text_data_json["payload"] - print(session_key) - print(changes) + changes = text_data_json["payload"]["changes"] + + if changes: + change_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id")) + # Channels that have been created on the client side won't exist on the server yet, so we need to add a special exception for them. + created_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id") and x.get("table") == CHANNEL and x.get("type") == CREATED) + # However, this would also give people a mechanism to edit existing channels on the server side by adding a channel create event for an + # already existing channel, so we have to filter out the channel ids that are already created on the server side, regardless of whether + # the user making the requests has permissions for those channels. + created_channel_ids = created_channel_ids.difference( + set(Channel.objects.filter(id__in=created_channel_ids).values_list("id", flat=True).distinct()) + ) + allowed_ids = set( + Channel.filter_edit_queryset(Channel.objects.filter(id__in=change_channel_ids), self.user).values_list("id", flat=True).distinct() + ).union(created_channel_ids) + # Allow changes that are either: + # Not related to a channel and instead related to the user if the user is the current user. + user_only_changes = [] + # Related to a channel that the user is an editor for. + channel_changes = [] + # Changes that cannot be made + disallowed_changes = [] + for c in changes: + if c.get("channel_id") is None and c.get("user_id") == self.user_id: + user_only_changes.append(c) + elif c.get("channel_id") in allowed_ids: + channel_changes.append(c) + else: + disallowed_changes.append(c) + change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) + if user_only_changes: + get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) + for channel_id in allowed_ids: + get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) + allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] + response_payload.update({"disallowed": disallowed_changes, "allowed": allowed_changes}) - # print(session_id) - # When the user sends somme changes then create async tasks and return to him - # {"disallowed": disallowed_changes, "allowed": allowed_changes} + self.send(json.dumps({ + 'response_payload': response_payload + })) From b656473f2b7cbeba656925d8e1869cc3649a012e Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 5 Jul 2022 22:59:41 +0530 Subject: [PATCH 09/47] FIX_unit_tests --- .../contentcuration/tests/test_websocket_consumer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index c5de7ede1f..4a4ae3d8be 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -12,7 +12,7 @@ class WebsocketTestCase(TransactionTestCase): - serialized_rollback = True + # serialized_rollback = True def setUp(self): call_command("loadconstants") From 1b9b63c3c05064c8d10a45f046b603b4d6da2c8e Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 7 Jul 2022 21:50:49 +0530 Subject: [PATCH 10/47] update redis version --- .github/workflows/pythontest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pythontest.yml b/.github/workflows/pythontest.yml index 7fdf6e178c..f9861d813f 100644 --- a/.github/workflows/pythontest.yml +++ b/.github/workflows/pythontest.yml @@ -44,7 +44,7 @@ jobs: # Label used to access the service container redis: # Docker Hub image - image: redis:4.0.9 + image: redis:6.0.9 # Set health checks to wait until redis has started options: >- --health-cmd "redis-cli ping" From b7a39f1141b68359f12a92ca4103a5e274cf74b0 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 8 Jul 2022 22:59:58 +0530 Subject: [PATCH 11/47] Add more unit tests --- .../tests/test_websocket_consumer.py | 73 +++++++++++++++++++ .../viewsets/websockets/consumers.py | 15 ++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index 4a4ae3d8be..043e0c2fa5 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -1,8 +1,10 @@ import os import pytest +from channels.layers import get_channel_layer from channels.testing import WebsocketCommunicator from django.core.management import call_command +from django.test import override_settings from django.test import TransactionTestCase from contentcuration.asgi import application @@ -29,3 +31,74 @@ async def test_authenticated_user_websocket_connection(self): connected = await communicator.connect() assert connected await communicator.disconnect() + + @pytest.mark.asyncio + async def test_unauthenticated_user_websocket_connection(self): + headers = [(b'cookie', self.client.cookies.output(attrs=["value"], header='', sep='; ').encode())] + communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) + connected, _ = await communicator.connect() + assert connected is False + await communicator.disconnect() + + @pytest.mark.asyncio + async def test_disconnect_websockets(self): + self.client.force_login(self.user) + headers = [(b'cookie', self.client.cookies.output(attrs=["value"], header='', sep='; ').encode())] + channel_layers_setting = { + "default": {"BACKEND": "channels.layers.InMemoryChannelLayer"} + } + with override_settings(CHANNEL_LAYERS=channel_layers_setting): + communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) + connected, _ = await communicator.connect() + channel_layer = get_channel_layer() + assert connected + await communicator.disconnect() + assert channel_layer.groups == {} + + @pytest.mark.asyncio + async def test_send_payload_websockets(self): + self.client.force_login(self.user) + headers = [(b'cookie', self.client.cookies.output(attrs=["value"], header='', sep='; ').encode())] + communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) + connected = await communicator.connect() + assert connected + await communicator.send_json_to({ + "payload": { + "changes": [ + { + "type": 2, + "key": "7ae83505f20a4642a004fadde7f151ed", + "table": "channel", + "rev": 253, + "channel_id": "7ae83505f20a4642a004fadde7f151ed", + "mods": { + "name": "test" + } + } + ], + "channel_revs": { + "7ae83505f20a4642a004fadde7f151ed": 51 + }, + "user_rev": 0 + }}) + response = await communicator.receive_json_from() + assert response["response_payload"] + await communicator.disconnect() + + @pytest.mark.asyncio + async def test_channels_groups(self): + self.client.force_login(self.user) + headers = [(b'cookie', self.client.cookies.output(attrs=["value"], header='', sep='; ').encode())] + channel_layers_setting = { + "default": {"BACKEND": "channels.layers.InMemoryChannelLayer"} + } + with override_settings(CHANNEL_LAYERS=channel_layers_setting): + communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) + connected, _ = await communicator.connect() + channel_layer = get_channel_layer() + print(channel_layer.groups) + # check the grou for channel exist + assert channel_layer.groups['12312312312123'] + assert channel_layer.groups[f"{self.user.id}"] + assert connected + await communicator.disconnect() diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 1b641dcb13..6d0a1912a8 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -1,6 +1,7 @@ import json from asgiref.sync import async_to_sync +from celery.utils.log import get_task_logger from channels.generic.websocket import WebsocketConsumer from contentcuration.models import Change @@ -9,6 +10,8 @@ from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.constants import CREATED +logger = get_task_logger(__name__) + class SyncConsumer(WebsocketConsumer): @@ -35,16 +38,16 @@ def connect(self): self.room_name = self.scope['url_route']['kwargs']['channel_id'] self.room_group_name = self.room_name - print("----------------------------------") - print("Connected to channel_id: " + self.room_name) - print("----------------------------------") + logger.info("----------------------------------") + logger.info("Connected to channel_id: " + self.room_name) + logger.info("----------------------------------") self.user = self.scope["user"] self.indiviual_room_group_name = str(self.user.id) - print("----------------------------------") - print("Connected to user " + str(self.user)) - print("----------------------------------") + logger.info("----------------------------------") + logger.info("Connected to user " + str(self.user)) + logger.info("----------------------------------") if self.check_authentication(): # Join room group based on channel_id From 1508d413f56dbf24192351051b3e5664e20969fe Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Sat, 16 Jul 2022 22:57:41 +0530 Subject: [PATCH 12/47] refine pr --- .../frontend/shared/data/serverSync.js | 18 +-- .../tests/test_websocket_consumer.py | 2 - .../viewsets/websockets/consumers.py | 104 +++++++++--------- 3 files changed, 64 insertions(+), 60 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 10f2791416..7fc52e0653 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -355,13 +355,17 @@ export function startSyncing() { cleanupLocks(); // Initiate a sync immediately in case any data // is left over in the database. - socket = new WebSocket( - 'ws://' + - window.location.host + - '/ws/sync_socket/' + - window.CHANNEL_EDIT_GLOBAL.channel_id + - '/' - ); + process.env.NODE_ENV == 'production' + ? (socket = new WebSocket( + new URL( + `wss://${window.location.host}/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/` + ) + )) + : (socket = new WebSocket( + new URL( + `ws://${window.location.host}/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/` + ) + )); // Connection opened socket.addEventListener('open', () => { console.log('Websocket connected'); diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index 043e0c2fa5..2ac6808e4e 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -14,7 +14,6 @@ class WebsocketTestCase(TransactionTestCase): - # serialized_rollback = True def setUp(self): call_command("loadconstants") @@ -96,7 +95,6 @@ async def test_channels_groups(self): communicator = WebsocketCommunicator(application, 'ws/sync_socket/12312312312123/', headers) connected, _ = await communicator.connect() channel_layer = get_channel_layer() - print(channel_layer.groups) # check the grou for channel exist assert channel_layer.groups['12312312312123'] assert channel_layer.groups[f"{self.user.id}"] diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 6d0a1912a8..d4c6790a2e 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -1,7 +1,7 @@ import json +import logging as logger from asgiref.sync import async_to_sync -from celery.utils.log import get_task_logger from channels.generic.websocket import WebsocketConsumer from contentcuration.models import Change @@ -10,7 +10,8 @@ from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.constants import CREATED -logger = get_task_logger(__name__) + +logging = logger.getLogger(__name__) class SyncConsumer(WebsocketConsumer): @@ -29,25 +30,22 @@ def __init__(self, *args, **kwargs): self.user_id = None self.indiviual_room_group_name = None - # This function gets executed when a user tries to make a websocket connection. - # - Creates and joins a group for indiviual user - # - Joins a public group based on channel_id provided in url def connect(self): - + """ + Executes when a user tries to make a websocket connection. + - Creates and joins a group for indiviual user + - Joins a public group based on channel_id provided in url + """ # Extract the channel_id from url self.room_name = self.scope['url_route']['kwargs']['channel_id'] self.room_group_name = self.room_name - logger.info("----------------------------------") - logger.info("Connected to channel_id: " + self.room_name) - logger.info("----------------------------------") + logging.debug("Connected to channel_id: " + self.room_name) self.user = self.scope["user"] self.indiviual_room_group_name = str(self.user.id) - logger.info("----------------------------------") - logger.info("Connected to user " + str(self.user)) - logger.info("----------------------------------") + logging.debug("Connected to user " + str(self.user)) if self.check_authentication(): # Join room group based on channel_id @@ -68,6 +66,9 @@ def connect(self): self.close() def disconnect(self, close_code): + """ + Executed to leave indiviual-user and channel group + """ # Leave channel_id room group async_to_sync(self.channel_layer.group_discard)( self.room_group_name, @@ -80,8 +81,10 @@ def disconnect(self, close_code): self.channel_name ) - # Receive message from WebSocket def receive(self, text_data): + """ + Executes when data is received from websocket + """ response_payload = { "disallowed": [], "allowed": [], @@ -91,41 +94,40 @@ def receive(self, text_data): text_data_json = json.loads(text_data) changes = text_data_json["payload"]["changes"] - if changes: - change_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id")) - # Channels that have been created on the client side won't exist on the server yet, so we need to add a special exception for them. - created_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id") and x.get("table") == CHANNEL and x.get("type") == CREATED) - # However, this would also give people a mechanism to edit existing channels on the server side by adding a channel create event for an - # already existing channel, so we have to filter out the channel ids that are already created on the server side, regardless of whether - # the user making the requests has permissions for those channels. - created_channel_ids = created_channel_ids.difference( - set(Channel.objects.filter(id__in=created_channel_ids).values_list("id", flat=True).distinct()) - ) - allowed_ids = set( - Channel.filter_edit_queryset(Channel.objects.filter(id__in=change_channel_ids), self.user).values_list("id", flat=True).distinct() - ).union(created_channel_ids) - # Allow changes that are either: - # Not related to a channel and instead related to the user if the user is the current user. - user_only_changes = [] - # Related to a channel that the user is an editor for. - channel_changes = [] - # Changes that cannot be made - disallowed_changes = [] - for c in changes: - if c.get("channel_id") is None and c.get("user_id") == self.user_id: - user_only_changes.append(c) - elif c.get("channel_id") in allowed_ids: - channel_changes.append(c) - else: - disallowed_changes.append(c) - change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) - if user_only_changes: - get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) - for channel_id in allowed_ids: - get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) - allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] - response_payload.update({"disallowed": disallowed_changes, "allowed": allowed_changes}) - - self.send(json.dumps({ - 'response_payload': response_payload - })) + change_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id")) + # Channels that have been created on the client side won't exist on the server yet, so we need to add a special exception for them. + created_channel_ids = set(x.get("channel_id") for x in changes if x.get("channel_id") and x.get("table") == CHANNEL and x.get("type") == CREATED) + # However, this would also give people a mechanism to edit existing channels on the server side by adding a channel create event for an + # already existing channel, so we have to filter out the channel ids that are already created on the server side, regardless of whether + # the user making the requests has permissions for those channels. + created_channel_ids = created_channel_ids.difference( + set(Channel.objects.filter(id__in=created_channel_ids).values_list("id", flat=True).distinct()) + ) + allowed_ids = set( + Channel.filter_edit_queryset(Channel.objects.filter(id__in=change_channel_ids), self.user).values_list("id", flat=True).distinct() + ).union(created_channel_ids) + # Allow changes that are either: + # Not related to a channel and instead related to the user if the user is the current user. + user_only_changes = [] + # Related to a channel that the user is an editor for. + channel_changes = [] + # Changes that cannot be made + disallowed_changes = [] + for c in changes: + if c.get("channel_id") is None and c.get("user_id") == self.user_id: + user_only_changes.append(c) + elif c.get("channel_id") in allowed_ids: + channel_changes.append(c) + else: + disallowed_changes.append(c) + change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) + if user_only_changes: + get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) + for channel_id in allowed_ids: + get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) + allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] + response_payload.update({"disallowed": disallowed_changes, "allowed": allowed_changes}) + + self.send(json.dumps({ + 'response_payload': response_payload + })) From 11ce747edabce0c2c0ed31adff3fabd6adc6922f Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 20 Jul 2022 01:06:06 +0530 Subject: [PATCH 13/47] refine PR --- contentcuration/contentcuration/asgi.py | 4 --- .../frontend/shared/data/serverSync.js | 21 ++++++------ .../viewsets/websockets/consumers.py | 33 +++++++++---------- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/contentcuration/contentcuration/asgi.py b/contentcuration/contentcuration/asgi.py index 75b1168d71..4973a69aef 100644 --- a/contentcuration/contentcuration/asgi.py +++ b/contentcuration/contentcuration/asgi.py @@ -1,13 +1,9 @@ -import os - from channels.auth import AuthMiddlewareStack from channels.routing import ProtocolTypeRouter from channels.routing import URLRouter from contentcuration.viewsets.websockets.routing import websocket_urlpatterns -os.environ.setdefault("DJANGO_SETTINGS_MODULE", "contentcuration.settings") - application = ProtocolTypeRouter({ "websocket": AuthMiddlewareStack( diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 7fc52e0653..af72b18fd9 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -355,22 +355,21 @@ export function startSyncing() { cleanupLocks(); // Initiate a sync immediately in case any data // is left over in the database. - process.env.NODE_ENV == 'production' - ? (socket = new WebSocket( - new URL( - `wss://${window.location.host}/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/` - ) - )) - : (socket = new WebSocket( - new URL( - `ws://${window.location.host}/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/` - ) - )); + + const websocketUrl = new URL( + `/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/`, + window.location.href + ); + websocketUrl.protocol = window.location.protocol == 'https:' ? 'wss:' : 'ws:'; + socket = new WebSocket(websocketUrl); + // Connection opened socket.addEventListener('open', () => { console.log('Websocket connected'); }); + debouncedSyncChanges(); + // Start the sync interval intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); db.on('changes', handleChanges); diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index d4c6790a2e..e525a7f0ea 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -15,21 +15,20 @@ class SyncConsumer(WebsocketConsumer): - - # Checks permissions - def check_authentication(self): - return self.scope['user'].is_authenticated - # Initial reset def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.room_name = None self.room_group_name = None - self.session_key = None - self.user = None - self.user_id = None self.indiviual_room_group_name = None + @property + def user(self): + return self.scope["user"] + + # Checks permissions + def check_authentication(self): + return self.user.is_authenticated + def connect(self): """ Executes when a user tries to make a websocket connection. @@ -37,12 +36,10 @@ def connect(self): - Joins a public group based on channel_id provided in url """ # Extract the channel_id from url - self.room_name = self.scope['url_route']['kwargs']['channel_id'] - self.room_group_name = self.room_name + self.room_group_name = self.scope['url_route']['kwargs']['channel_id'] - logging.debug("Connected to channel_id: " + self.room_name) + logging.debug("Connected to channel_id: " + self.room_group_name) - self.user = self.scope["user"] self.indiviual_room_group_name = str(self.user.id) logging.debug("Connected to user " + str(self.user)) @@ -89,8 +86,8 @@ def receive(self, text_data): "disallowed": [], "allowed": [], } - self.user_id = self.scope['user'].id - self.session_key = self.scope['cookies']['kolibri_studio_sessionid'] + user_id = self.user.id + session_key = self.scope['cookies']['kolibri_studio_sessionid'] text_data_json = json.loads(text_data) changes = text_data_json["payload"]["changes"] @@ -114,15 +111,15 @@ def receive(self, text_data): # Changes that cannot be made disallowed_changes = [] for c in changes: - if c.get("channel_id") is None and c.get("user_id") == self.user_id: + if c.get("channel_id") is None and c.get("user_id") == user_id: user_only_changes.append(c) elif c.get("channel_id") in allowed_ids: channel_changes.append(c) else: disallowed_changes.append(c) - change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) + change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=user_id, session_key=session_key) if user_only_changes: - get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) + get_or_create_async_task("apply_user_changes", self.user, user_id=user_id) for channel_id in allowed_ids: get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] From 721a052832336a29c4babe767a63322d52aee932 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 15 Jul 2022 17:54:32 +0530 Subject: [PATCH 14/47] setup django signals for change object --- contentcuration/contentcuration/apps.py | 2 + .../viewsets/websockets/consumers.py | 15 +++++-- .../viewsets/websockets/signals.py | 40 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 contentcuration/contentcuration/viewsets/websockets/signals.py diff --git a/contentcuration/contentcuration/apps.py b/contentcuration/contentcuration/apps.py index f42624ce9e..12622c2403 100644 --- a/contentcuration/contentcuration/apps.py +++ b/contentcuration/contentcuration/apps.py @@ -8,6 +8,8 @@ class ContentConfig(AppConfig): name = 'contentcuration' def ready(self): + # signals for websockets + import contentcuration.viewsets.websockets.signals # see note in the celery_signals.py file for why we import here. import contentcuration.utils.celery.signals # noqa diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index e525a7f0ea..2e20a221c4 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -111,15 +111,15 @@ def receive(self, text_data): # Changes that cannot be made disallowed_changes = [] for c in changes: - if c.get("channel_id") is None and c.get("user_id") == user_id: + if c.get("channel_id") is None and c.get("user_id") == self.user_id: user_only_changes.append(c) elif c.get("channel_id") in allowed_ids: channel_changes.append(c) else: disallowed_changes.append(c) - change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=user_id, session_key=session_key) + change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) if user_only_changes: - get_or_create_async_task("apply_user_changes", self.user, user_id=user_id) + get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) for channel_id in allowed_ids: get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] @@ -128,3 +128,12 @@ def receive(self, text_data): self.send(json.dumps({ 'response_payload': response_payload })) + + # Receive message from room group + def broadcast_changes(self, event): + change = event['change'] + + # Send message to WebSocket + self.send(text_data=json.dumps({ + 'change': change + })) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py new file mode 100644 index 0000000000..d4ecf41b8a --- /dev/null +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -0,0 +1,40 @@ +from asgiref.sync import async_to_sync +from channels.layers import get_channel_layer +from django.db.models.signals import post_save +from django.dispatch import receiver + +from contentcuration.models import Change + + +@receiver(post_save, sender=Change, weak=False) +def broadcast_new_change_model(sender, instance, created, **kwargs): + channel_layer = get_channel_layer() + print(instance.__dict__) + serialized_change_object = Change.serialize(instance) + room_group_name = instance.channel_id + indiviual_room_group_name = instance.user_id + # if the change is only related to user we broadcast changes only to user + if not indiviual_room_group_name and room_group_name: + async_to_sync(channel_layer.group_send)( + room_group_name, + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + elif indiviual_room_group_name and not room_group_name: + async_to_sync(channel_layer.group_send)( + indiviual_room_group_name, + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + + async_to_sync(channel_layer.group_send)( + room_group_name, + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) From ad1461aab712bf34c19e1ea3d6d02e6e736c88bf Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 18 Jul 2022 21:40:20 +0530 Subject: [PATCH 15/47] properly handle cases in signals --- .../viewsets/websockets/signals.py | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index d4ecf41b8a..adc341e890 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -11,9 +11,22 @@ def broadcast_new_change_model(sender, instance, created, **kwargs): channel_layer = get_channel_layer() print(instance.__dict__) serialized_change_object = Change.serialize(instance) + # Name of channel group room_group_name = instance.channel_id + # name of indiviual_user group indiviual_room_group_name = instance.user_id - # if the change is only related to user we broadcast changes only to user + + # if the change object is errored then we broadcast the info back to indiviual user + if instance.errored is True: + async_to_sync(channel_layer.group_send)( + instance.created_by_id, + { + 'type': 'broadcast_changes', + 'errored': serialized_change_object + } + ) + + # if the change is related to channel we broadcast changes to channel group if not indiviual_room_group_name and room_group_name: async_to_sync(channel_layer.group_send)( room_group_name, @@ -22,6 +35,7 @@ def broadcast_new_change_model(sender, instance, created, **kwargs): 'change': serialized_change_object } ) + # if the change is only related to indiviual user elif indiviual_room_group_name and not room_group_name: async_to_sync(channel_layer.group_send)( indiviual_room_group_name, @@ -30,11 +44,19 @@ def broadcast_new_change_model(sender, instance, created, **kwargs): 'change': serialized_change_object } ) - - async_to_sync(channel_layer.group_send)( - room_group_name, - { - 'type': 'broadcast_changes', - 'change': serialized_change_object - } - ) + # if the change is realted to both user and channel then we will broadcast to both of the groups + elif indiviual_room_group_name and room_group_name: + async_to_sync(channel_layer.group_send)( + room_group_name, + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + async_to_sync(channel_layer.group_send)( + indiviual_room_group_name, + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) From 279daf47914bc5a6bb6c5c27a14aa762a50f6778 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 2 Aug 2022 12:50:00 +0530 Subject: [PATCH 16/47] complete tests for signals --- .../tests/test_change_signals.py | 117 ++++++++++++++++++ .../tests/test_websocket_consumer.py | 1 - .../tests/viewsets/test_contentnode.py | 1 - .../viewsets/websockets/signals.py | 9 +- 4 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 contentcuration/contentcuration/tests/test_change_signals.py diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py new file mode 100644 index 0000000000..6911e4993d --- /dev/null +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -0,0 +1,117 @@ +import uuid + +from django.core.management import call_command +from django.test import TestCase +from mock import patch + +from contentcuration.models import Change +from contentcuration.tests import testdata +from contentcuration.tests.viewsets.base import generate_create_event +from contentcuration.tests.viewsets.base import generate_update_event +from contentcuration.viewsets.sync.constants import BOOKMARK +from contentcuration.viewsets.sync.constants import CHANNEL +from contentcuration.viewsets.sync.constants import EDITOR_M2M + + +class ChangeSignalTestCase(TestCase): + def setUp(self): + call_command("loadconstants") + self.user = testdata.user("mrtest@testy.com") + self.channel = testdata.channel() + self.channel.editors.add(self.user) + + def tearDown(self): + self.user.delete() + + @property + def channel_metadata(self): + return { + "name": "ozer's cool channel", + "id": uuid.uuid4().hex, + "description": "coolest channel that side of the Pacific", + } + + @property + def bookmark_metadata(self): + return { + "channel": self.channel.id, + } + + @patch('contentcuration.viewsets.websockets.signals.broadcast_new_change_model') + def test_change_signal_handler(self, mock_signal): + """ + Test if signal is getting triggered when an change object gets created. + """ + self.client.force_login(self.user) + new_name = "This is not the old name" + Change.create_change(generate_update_event(self.channel.id, CHANNEL, {"name": new_name}, channel_id=self.channel.id)) + assert mock_signal.call_count == 1 + + @patch('contentcuration.viewsets.websockets.signals.async_to_sync') + @patch('contentcuration.viewsets.websockets.signals.get_channel_layer') + def test_signal_handler_channel_specific_changes(self, mock_get_channel_layer, mock_async_to_sync): + """ + Test changes that are specific to channel(change channel name) only. + """ + new_name = "This is not the old name" + change_obj = Change.create_change(generate_update_event(self.channel.id, CHANNEL, {"name": new_name}, channel_id=self.channel.id)) + + change_serialized = Change.serialize(change_obj) + + channel_layer = mock_get_channel_layer.return_value + mock_async_to_sync.assert_called_once_with(channel_layer.group_send) + async_mock_return_value = mock_async_to_sync.return_value + async_mock_return_value.assert_called_once_with(self.channel.id, { + 'type': 'broadcast_changes', + 'change': change_serialized + }) + + @patch('contentcuration.viewsets.websockets.signals.async_to_sync') + @patch('contentcuration.viewsets.websockets.signals.get_channel_layer') + def test_signal_handler_user_specific_changes(self, mock_get_channel_layer, mock_async_to_sync): + """ + Test changes that are specific to user(bookmarks) only. + """ + self.client.force_login(user=self.user) + bookmark = self.bookmark_metadata + change_obj = Change.create_change(generate_create_event( + bookmark["channel"], + BOOKMARK, + bookmark, + user_id=self.user.id, + )) + change_serialized = Change.serialize(change_obj) + channel_layer = mock_get_channel_layer.return_value + mock_async_to_sync.assert_called_with(channel_layer.group_send) + async_mock_return_value = mock_async_to_sync.return_value + async_mock_return_value.assert_called_with(self.user.id, { + 'type': 'broadcast_changes', + 'change': change_serialized + }) + + @patch('contentcuration.viewsets.websockets.signals.async_to_sync') + @patch('contentcuration.viewsets.websockets.signals.get_channel_layer') + def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer, mock_async_to_sync): + """ + Test changes that are common to both channel and user(invitations). + """ + editor = self.user + self.client.force_login(self.user) + change_obj = Change.create_change(generate_create_event([editor.id, self.channel.id], EDITOR_M2M, {}, channel_id=self.channel.id, user_id=editor.id)) + change_serialized = Change.serialize(change_obj) + channel_layer = mock_get_channel_layer.return_value + mock_async_to_sync.assert_called_with(channel_layer.group_send) + async_mock_return_value = mock_async_to_sync.return_value + async_mock_return_value.assert_called_with(editor.id, { + 'type': 'broadcast_changes', + 'change': change_serialized + }) + assert 2 == mock_async_to_sync.call_count + async_mock_return_value.assert_any_call(self.channel.id, { + 'type': 'broadcast_changes', + 'change': change_serialized + }) + async_mock_return_value.assert_any_call(editor.id, { + 'type': 'broadcast_changes', + 'change': change_serialized + }) diff --git a/contentcuration/contentcuration/tests/test_websocket_consumer.py b/contentcuration/contentcuration/tests/test_websocket_consumer.py index 2ac6808e4e..73ae2deccc 100644 --- a/contentcuration/contentcuration/tests/test_websocket_consumer.py +++ b/contentcuration/contentcuration/tests/test_websocket_consumer.py @@ -14,7 +14,6 @@ class WebsocketTestCase(TransactionTestCase): - def setUp(self): call_command("loadconstants") self.user = testdata.user("mrtest@testy.com") diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index 2a3f966add..b8ddcd0d0d 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -374,7 +374,6 @@ def test_consolidate_extra_fields(self): self.viewset_url(pk=contentnode.id), format="json", ) self.assertEqual(response.status_code, 200, response.content) - print(response.data["extra_fields"]) self.assertEqual(response.data["extra_fields"]["options"]["completion_criteria"]["threshold"]["m"], 3) self.assertEqual(response.data["extra_fields"]["options"]["completion_criteria"]["threshold"]["n"], 6) self.assertEqual(response.data["extra_fields"]["options"]["completion_criteria"]["threshold"]["mastery_model"], exercises.M_OF_N) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index adc341e890..7542e8f383 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -7,9 +7,12 @@ @receiver(post_save, sender=Change, weak=False) -def broadcast_new_change_model(sender, instance, created, **kwargs): +def broadcast_new_change_model_handler(sender, instance, created, **kwargs): + broadcast_new_change_model(instance) + + +def broadcast_new_change_model(instance): channel_layer = get_channel_layer() - print(instance.__dict__) serialized_change_object = Change.serialize(instance) # Name of channel group room_group_name = instance.channel_id @@ -17,7 +20,7 @@ def broadcast_new_change_model(sender, instance, created, **kwargs): indiviual_room_group_name = instance.user_id # if the change object is errored then we broadcast the info back to indiviual user - if instance.errored is True: + if instance.errored: async_to_sync(channel_layer.group_send)( instance.created_by_id, { From 40a08fecb1ed9368fc5da6d8375d1c43a0b842c6 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 2 Aug 2022 13:19:10 +0530 Subject: [PATCH 17/47] fix rebase --- .../contentcuration/viewsets/websockets/consumers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 2e20a221c4..0baca3cb5b 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -111,15 +111,15 @@ def receive(self, text_data): # Changes that cannot be made disallowed_changes = [] for c in changes: - if c.get("channel_id") is None and c.get("user_id") == self.user_id: + if c.get("channel_id") is None and c.get("user_id") == user_id: user_only_changes.append(c) elif c.get("channel_id") in allowed_ids: channel_changes.append(c) else: disallowed_changes.append(c) - change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=self.user_id, session_key=self.session_key) + change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=user_id, session_key=session_key) if user_only_changes: - get_or_create_async_task("apply_user_changes", self.user, user_id=self.user_id) + get_or_create_async_task("apply_user_changes", self.user, user_id=user_id) for channel_id in allowed_ids: get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] From aa54a249674b1786d8f5699b0e8d5fd6eb3e2619 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 2 Aug 2022 20:58:56 +0530 Subject: [PATCH 18/47] broken test_cannot_create_files --- .../contentcuration/tests/test_change_signals.py | 7 ++++++- .../contentcuration/viewsets/websockets/signals.py | 8 ++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index 6911e4993d..f50ec10140 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -6,6 +6,7 @@ from contentcuration.models import Change from contentcuration.tests import testdata +from contentcuration.tests.base import BucketTestMixin from contentcuration.tests.viewsets.base import generate_create_event from contentcuration.tests.viewsets.base import generate_update_event from contentcuration.viewsets.sync.constants import BOOKMARK @@ -13,14 +14,18 @@ from contentcuration.viewsets.sync.constants import EDITOR_M2M -class ChangeSignalTestCase(TestCase): +class ChangeSignalTestCase(TestCase, BucketTestMixin): def setUp(self): call_command("loadconstants") + if not self.persist_bucket: + self.create_bucket() self.user = testdata.user("mrtest@testy.com") self.channel = testdata.channel() self.channel.editors.add(self.user) def tearDown(self): + if not self.persist_bucket: + self.delete_bucket() self.user.delete() @property diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index 7542e8f383..edecdaf90a 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -15,9 +15,13 @@ def broadcast_new_change_model(instance): channel_layer = get_channel_layer() serialized_change_object = Change.serialize(instance) # Name of channel group - room_group_name = instance.channel_id + room_group_name = str(instance.channel_id or "dummy") + # name of indiviual_user group - indiviual_room_group_name = instance.user_id + indiviual_room_group_name = str(instance.user_id or "dummy") + + print("DEBUG", room_group_name) + print("DEBUG", indiviual_room_group_name) # if the change object is errored then we broadcast the info back to indiviual user if instance.errored: From 4092a3fb5db42f7e4c2ab4b4c56d6a140413a95c Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Tue, 2 Aug 2022 23:09:01 +0530 Subject: [PATCH 19/47] fix typecasting --- .../tests/test_change_signals.py | 6 +++--- .../viewsets/websockets/signals.py | 17 +++++++---------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index f50ec10140..3f079ff02c 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -89,7 +89,7 @@ def test_signal_handler_user_specific_changes(self, mock_get_channel_layer, mock channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_with(self.user.id, { + async_mock_return_value.assert_called_with(str(self.user.id), { 'type': 'broadcast_changes', 'change': change_serialized }) @@ -107,7 +107,7 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_with(editor.id, { + async_mock_return_value.assert_called_with(str(editor.id), { 'type': 'broadcast_changes', 'change': change_serialized }) @@ -116,7 +116,7 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer 'type': 'broadcast_changes', 'change': change_serialized }) - async_mock_return_value.assert_any_call(editor.id, { + async_mock_return_value.assert_any_call(str(editor.id), { 'type': 'broadcast_changes', 'change': change_serialized }) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index edecdaf90a..4b853ca12d 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -15,18 +15,15 @@ def broadcast_new_change_model(instance): channel_layer = get_channel_layer() serialized_change_object = Change.serialize(instance) # Name of channel group - room_group_name = str(instance.channel_id or "dummy") + room_group_name = instance.channel_id # name of indiviual_user group - indiviual_room_group_name = str(instance.user_id or "dummy") - - print("DEBUG", room_group_name) - print("DEBUG", indiviual_room_group_name) + indiviual_room_group_name = instance.user_id # if the change object is errored then we broadcast the info back to indiviual user if instance.errored: async_to_sync(channel_layer.group_send)( - instance.created_by_id, + str(instance.created_by_id or ""), { 'type': 'broadcast_changes', 'errored': serialized_change_object @@ -36,7 +33,7 @@ def broadcast_new_change_model(instance): # if the change is related to channel we broadcast changes to channel group if not indiviual_room_group_name and room_group_name: async_to_sync(channel_layer.group_send)( - room_group_name, + str(room_group_name or ""), { 'type': 'broadcast_changes', 'change': serialized_change_object @@ -45,7 +42,7 @@ def broadcast_new_change_model(instance): # if the change is only related to indiviual user elif indiviual_room_group_name and not room_group_name: async_to_sync(channel_layer.group_send)( - indiviual_room_group_name, + str(indiviual_room_group_name or ""), { 'type': 'broadcast_changes', 'change': serialized_change_object @@ -54,14 +51,14 @@ def broadcast_new_change_model(instance): # if the change is realted to both user and channel then we will broadcast to both of the groups elif indiviual_room_group_name and room_group_name: async_to_sync(channel_layer.group_send)( - room_group_name, + str(room_group_name or ""), { 'type': 'broadcast_changes', 'change': serialized_change_object } ) async_to_sync(channel_layer.group_send)( - indiviual_room_group_name, + str(indiviual_room_group_name or ""), { 'type': 'broadcast_changes', 'change': serialized_change_object From cc9982a7057dc96a2e6273a2d3d5c4caef4462c9 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 5 Aug 2022 15:09:17 +0530 Subject: [PATCH 20/47] add helper functions for tests --- .../tests/test_change_signals.py | 60 ++++++--------- .../contentcuration/utils/websocket_helper.py | 71 ++++++++++++++++++ .../viewsets/websockets/signals.py | 73 ++++++++++--------- 3 files changed, 132 insertions(+), 72 deletions(-) create mode 100644 contentcuration/contentcuration/utils/websocket_helper.py diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index 3f079ff02c..2e44b413df 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -1,5 +1,3 @@ -import uuid - from django.core.management import call_command from django.test import TestCase from mock import patch @@ -7,11 +5,12 @@ from contentcuration.models import Change from contentcuration.tests import testdata from contentcuration.tests.base import BucketTestMixin -from contentcuration.tests.viewsets.base import generate_create_event from contentcuration.tests.viewsets.base import generate_update_event -from contentcuration.viewsets.sync.constants import BOOKMARK +from contentcuration.utils.websocket_helper import create_channel_specific_change_object +from contentcuration.utils.websocket_helper import create_channel_user_common_change_object +from contentcuration.utils.websocket_helper import create_errored_change_object +from contentcuration.utils.websocket_helper import create_user_specific_change_object from contentcuration.viewsets.sync.constants import CHANNEL -from contentcuration.viewsets.sync.constants import EDITOR_M2M class ChangeSignalTestCase(TestCase, BucketTestMixin): @@ -22,26 +21,13 @@ def setUp(self): self.user = testdata.user("mrtest@testy.com") self.channel = testdata.channel() self.channel.editors.add(self.user) + self.client.force_login(user=self.user) def tearDown(self): if not self.persist_bucket: self.delete_bucket() self.user.delete() - @property - def channel_metadata(self): - return { - "name": "ozer's cool channel", - "id": uuid.uuid4().hex, - "description": "coolest channel that side of the Pacific", - } - - @property - def bookmark_metadata(self): - return { - "channel": self.channel.id, - } - @patch('contentcuration.viewsets.websockets.signals.broadcast_new_change_model') def test_change_signal_handler(self, mock_signal): """ @@ -58,11 +44,7 @@ def test_signal_handler_channel_specific_changes(self, mock_get_channel_layer, m """ Test changes that are specific to channel(change channel name) only. """ - new_name = "This is not the old name" - change_obj = Change.create_change(generate_update_event(self.channel.id, CHANNEL, {"name": new_name}, channel_id=self.channel.id)) - - change_serialized = Change.serialize(change_obj) - + change_serialized = create_channel_specific_change_object(self.channel) channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_once_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value @@ -77,15 +59,7 @@ def test_signal_handler_user_specific_changes(self, mock_get_channel_layer, mock """ Test changes that are specific to user(bookmarks) only. """ - self.client.force_login(user=self.user) - bookmark = self.bookmark_metadata - change_obj = Change.create_change(generate_create_event( - bookmark["channel"], - BOOKMARK, - bookmark, - user_id=self.user.id, - )) - change_serialized = Change.serialize(change_obj) + change_serialized = create_user_specific_change_object(self.user, self.channel) channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value @@ -101,9 +75,7 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer Test changes that are common to both channel and user(invitations). """ editor = self.user - self.client.force_login(self.user) - change_obj = Change.create_change(generate_create_event([editor.id, self.channel.id], EDITOR_M2M, {}, channel_id=self.channel.id, user_id=editor.id)) - change_serialized = Change.serialize(change_obj) + change_serialized = create_channel_user_common_change_object(editor, self.channel) channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value @@ -120,3 +92,19 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer 'type': 'broadcast_changes', 'change': change_serialized }) + + @patch('contentcuration.viewsets.websockets.signals.async_to_sync') + @patch('contentcuration.viewsets.websockets.signals.get_channel_layer') + def test_signal_handler_errored_changes(self, mock_get_channel_layer, mock_async_to_sync): + """ + Test changes that are errored! + """ + change_serialized = create_errored_change_object(self.user, self.channel) + channel_layer = mock_get_channel_layer.return_value + mock_async_to_sync.assert_called_with(channel_layer.group_send) + async_mock_return_value = mock_async_to_sync.return_value + assert 1 == mock_async_to_sync.call_count + async_mock_return_value.assert_any_call(str(self.user.id), { + 'type': 'broadcast_changes', + 'errored': change_serialized + }) diff --git a/contentcuration/contentcuration/utils/websocket_helper.py b/contentcuration/contentcuration/utils/websocket_helper.py new file mode 100644 index 0000000000..b42d267bcd --- /dev/null +++ b/contentcuration/contentcuration/utils/websocket_helper.py @@ -0,0 +1,71 @@ +import uuid + +from le_utils.constants import content_kinds + +from contentcuration import models +from contentcuration.models import Change +from contentcuration.tests.viewsets.base import generate_create_event +from contentcuration.tests.viewsets.base import generate_update_event +from contentcuration.viewsets.sync.constants import BOOKMARK +from contentcuration.viewsets.sync.constants import CHANNEL +from contentcuration.viewsets.sync.constants import CONTENTNODE +from contentcuration.viewsets.sync.constants import EDITOR_M2M + + +def bookmark_metadata(channel): + return { + "channel": channel.id, + } + + +def contentnode_db_metadata(channel): + return { + "title": "Aron's cool contentnode", + "id": uuid.uuid4().hex, + "kind_id": content_kinds.VIDEO, + "description": "coolest contentnode this side of the Pacific", + "parent_id": channel.main_tree_id, + } + + +def create_user_specific_change_object(user, channel): + bookmark = bookmark_metadata(channel) + change_obj = Change.create_change(generate_create_event( + bookmark["channel"], + BOOKMARK, + bookmark, + user_id=user.id, + )) + change_obj.applied = True + change_obj.save() + change_serialized = Change.serialize(change_obj) + return change_serialized + + +def create_channel_specific_change_object(channel): + new_name = "This is not the old name" + change_obj = Change.create_change(generate_update_event(channel.id, CHANNEL, {"name": new_name}, channel_id=channel.id)) + change_obj.applied = True + change_obj.save() + change_serialized = Change.serialize(change_obj) + return change_serialized + + +def create_channel_user_common_change_object(user, channel): + editor = user + change_obj = Change.create_change(generate_create_event([editor.id, channel.id], EDITOR_M2M, {}, channel_id=channel.id, user_id=editor.id)) + change_obj.applied = True + change_obj.save() + change_serialized = Change.serialize(change_obj) + return change_serialized + + +def create_errored_change_object(user, channel): + contentnode = models.ContentNode.objects.create(**contentnode_db_metadata(channel)) + tag = "howzat!" + change_obj = Change.create_change(generate_update_event(contentnode.id, CONTENTNODE, { + "tags": [tag]}, channel_id=channel.id), created_by_id=user.id) + change_obj.errored = True + change_obj.save() + change_serialized = Change.serialize(change_obj) + return change_serialized diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index 4b853ca12d..a298c5193e 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -19,7 +19,8 @@ def broadcast_new_change_model(instance): # name of indiviual_user group indiviual_room_group_name = instance.user_id - + print("DEBUG", instance.errored) + print("DEBUG", instance.__dict__) # if the change object is errored then we broadcast the info back to indiviual user if instance.errored: async_to_sync(channel_layer.group_send)( @@ -29,38 +30,38 @@ def broadcast_new_change_model(instance): 'errored': serialized_change_object } ) - - # if the change is related to channel we broadcast changes to channel group - if not indiviual_room_group_name and room_group_name: - async_to_sync(channel_layer.group_send)( - str(room_group_name or ""), - { - 'type': 'broadcast_changes', - 'change': serialized_change_object - } - ) - # if the change is only related to indiviual user - elif indiviual_room_group_name and not room_group_name: - async_to_sync(channel_layer.group_send)( - str(indiviual_room_group_name or ""), - { - 'type': 'broadcast_changes', - 'change': serialized_change_object - } - ) - # if the change is realted to both user and channel then we will broadcast to both of the groups - elif indiviual_room_group_name and room_group_name: - async_to_sync(channel_layer.group_send)( - str(room_group_name or ""), - { - 'type': 'broadcast_changes', - 'change': serialized_change_object - } - ) - async_to_sync(channel_layer.group_send)( - str(indiviual_room_group_name or ""), - { - 'type': 'broadcast_changes', - 'change': serialized_change_object - } - ) + if instance.applied: + # if the change is related to channel we broadcast changes to channel group + if not indiviual_room_group_name and room_group_name: + async_to_sync(channel_layer.group_send)( + str(room_group_name or ""), + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + # if the change is only related to indiviual user + elif indiviual_room_group_name and not room_group_name: + async_to_sync(channel_layer.group_send)( + str(indiviual_room_group_name or ""), + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + # if the change is realted to both user and channel then we will broadcast to both of the groups + elif indiviual_room_group_name and room_group_name: + async_to_sync(channel_layer.group_send)( + str(room_group_name or ""), + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) + async_to_sync(channel_layer.group_send)( + str(indiviual_room_group_name or ""), + { + 'type': 'broadcast_changes', + 'change': serialized_change_object + } + ) From e9bc522b1ba99e5e58341fdc243d749ba7ad7d32 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Sun, 7 Aug 2022 13:14:45 +0530 Subject: [PATCH 21/47] refactored tests --- .../tests/test_change_signals.py | 15 ++++------ .../contentcuration/utils/websocket_helper.py | 28 ++++++++++++++++--- .../viewsets/websockets/consumers.py | 4 ++- .../viewsets/websockets/signals.py | 26 ++++++++++++----- 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index 2e44b413df..5c7ea7618a 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -44,7 +44,7 @@ def test_signal_handler_channel_specific_changes(self, mock_get_channel_layer, m """ Test changes that are specific to channel(change channel name) only. """ - change_serialized = create_channel_specific_change_object(self.channel) + change_serialized = create_channel_specific_change_object(self.user, self.channel) channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_once_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value @@ -61,9 +61,9 @@ def test_signal_handler_user_specific_changes(self, mock_get_channel_layer, mock """ change_serialized = create_user_specific_change_object(self.user, self.channel) channel_layer = mock_get_channel_layer.return_value - mock_async_to_sync.assert_called_with(channel_layer.group_send) + mock_async_to_sync.assert_called_once_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_with(str(self.user.id), { + async_mock_return_value.assert_called_once_with(str(self.user.id), { 'type': 'broadcast_changes', 'change': change_serialized }) @@ -77,12 +77,9 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer editor = self.user change_serialized = create_channel_user_common_change_object(editor, self.channel) channel_layer = mock_get_channel_layer.return_value + assert 2 == mock_async_to_sync.call_count mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_with(str(editor.id), { - 'type': 'broadcast_changes', - 'change': change_serialized - }) assert 2 == mock_async_to_sync.call_count async_mock_return_value.assert_any_call(self.channel.id, { 'type': 'broadcast_changes', @@ -101,10 +98,10 @@ def test_signal_handler_errored_changes(self, mock_get_channel_layer, mock_async """ change_serialized = create_errored_change_object(self.user, self.channel) channel_layer = mock_get_channel_layer.return_value - mock_async_to_sync.assert_called_with(channel_layer.group_send) + mock_async_to_sync.assert_called_once_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value assert 1 == mock_async_to_sync.call_count - async_mock_return_value.assert_any_call(str(self.user.id), { + async_mock_return_value.assert_called_once_with(str(self.user.id), { 'type': 'broadcast_changes', 'errored': change_serialized }) diff --git a/contentcuration/contentcuration/utils/websocket_helper.py b/contentcuration/contentcuration/utils/websocket_helper.py index b42d267bcd..2116511e9b 100644 --- a/contentcuration/contentcuration/utils/websocket_helper.py +++ b/contentcuration/contentcuration/utils/websocket_helper.py @@ -12,6 +12,25 @@ from contentcuration.viewsets.sync.constants import EDITOR_M2M +class NoneCreatedByIdError(Exception): + """ + Use to log change object whose created_by_id. We don't raise this error, + just feed it to Sentry for reporting. + """ + + def __init__(self, instance): + + self.change_object = instance + message = ( + "The change object did not have a created_by_id {}" + ) + self.message = message.format( + instance.__dict__ + ) + + super(NoneCreatedByIdError, self).__init__(self.message) + + def bookmark_metadata(channel): return { "channel": channel.id, @@ -35,16 +54,16 @@ def create_user_specific_change_object(user, channel): BOOKMARK, bookmark, user_id=user.id, - )) + ), created_by_id=user.id) change_obj.applied = True change_obj.save() change_serialized = Change.serialize(change_obj) return change_serialized -def create_channel_specific_change_object(channel): +def create_channel_specific_change_object(user, channel): new_name = "This is not the old name" - change_obj = Change.create_change(generate_update_event(channel.id, CHANNEL, {"name": new_name}, channel_id=channel.id)) + change_obj = Change.create_change(generate_update_event(channel.id, CHANNEL, {"name": new_name}, channel_id=channel.id), created_by_id=user.id) change_obj.applied = True change_obj.save() change_serialized = Change.serialize(change_obj) @@ -53,7 +72,8 @@ def create_channel_specific_change_object(channel): def create_channel_user_common_change_object(user, channel): editor = user - change_obj = Change.create_change(generate_create_event([editor.id, channel.id], EDITOR_M2M, {}, channel_id=channel.id, user_id=editor.id)) + change_obj = Change.create_change(generate_create_event([editor.id, channel.id], EDITOR_M2M, {}, + channel_id=channel.id, user_id=editor.id), created_by_id=user.id) change_obj.applied = True change_obj.save() change_serialized = Change.serialize(change_obj) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 0baca3cb5b..1a159818c5 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -129,8 +129,10 @@ def receive(self, text_data): 'response_payload': response_payload })) - # Receive message from room group def broadcast_changes(self, event): + """ + Receive message events sent to the subscribed groups from our Django signal handlers, and relay the messages to the frontend + """ change = event['change'] # Send message to WebSocket diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index a298c5193e..a0af53db7e 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -1,9 +1,15 @@ +import logging as logger + from asgiref.sync import async_to_sync from channels.layers import get_channel_layer from django.db.models.signals import post_save from django.dispatch import receiver from contentcuration.models import Change +from contentcuration.utils.sentry import report_exception +from contentcuration.utils.websocket_helper import NoneCreatedByIdError + +logging = logger.getLogger(__name__) @receiver(post_save, sender=Change, weak=False) @@ -19,12 +25,18 @@ def broadcast_new_change_model(instance): # name of indiviual_user group indiviual_room_group_name = instance.user_id - print("DEBUG", instance.errored) - print("DEBUG", instance.__dict__) + + if(instance.created_by_id is None): + try: + raise NoneCreatedByIdError(instance) + except NoneCreatedByIdError as e: + report_exception(e) + return + # if the change object is errored then we broadcast the info back to indiviual user if instance.errored: async_to_sync(channel_layer.group_send)( - str(instance.created_by_id or ""), + str(instance.created_by_id), { 'type': 'broadcast_changes', 'errored': serialized_change_object @@ -34,7 +46,7 @@ def broadcast_new_change_model(instance): # if the change is related to channel we broadcast changes to channel group if not indiviual_room_group_name and room_group_name: async_to_sync(channel_layer.group_send)( - str(room_group_name or ""), + str(room_group_name), { 'type': 'broadcast_changes', 'change': serialized_change_object @@ -43,7 +55,7 @@ def broadcast_new_change_model(instance): # if the change is only related to indiviual user elif indiviual_room_group_name and not room_group_name: async_to_sync(channel_layer.group_send)( - str(indiviual_room_group_name or ""), + str(indiviual_room_group_name), { 'type': 'broadcast_changes', 'change': serialized_change_object @@ -52,14 +64,14 @@ def broadcast_new_change_model(instance): # if the change is realted to both user and channel then we will broadcast to both of the groups elif indiviual_room_group_name and room_group_name: async_to_sync(channel_layer.group_send)( - str(room_group_name or ""), + str(room_group_name), { 'type': 'broadcast_changes', 'change': serialized_change_object } ) async_to_sync(channel_layer.group_send)( - str(indiviual_room_group_name or ""), + str(indiviual_room_group_name), { 'type': 'broadcast_changes', 'change': serialized_change_object From afe273a2b2bbfcc05e9c3289285f22af6cc47a09 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Sun, 7 Aug 2022 14:00:14 +0530 Subject: [PATCH 22/47] fix typos --- contentcuration/contentcuration/utils/websocket_helper.py | 2 +- contentcuration/contentcuration/viewsets/websockets/signals.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/utils/websocket_helper.py b/contentcuration/contentcuration/utils/websocket_helper.py index 2116511e9b..485c64cd4f 100644 --- a/contentcuration/contentcuration/utils/websocket_helper.py +++ b/contentcuration/contentcuration/utils/websocket_helper.py @@ -14,7 +14,7 @@ class NoneCreatedByIdError(Exception): """ - Use to log change object whose created_by_id. We don't raise this error, + Use to log change object whose created_by_id is set to none. We don't raise this error, just feed it to Sentry for reporting. """ diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index a0af53db7e..43c81bfdaf 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -31,6 +31,7 @@ def broadcast_new_change_model(instance): raise NoneCreatedByIdError(instance) except NoneCreatedByIdError as e: report_exception(e) + logging.error("Missing expected Change.created_by_id") return # if the change object is errored then we broadcast the info back to indiviual user From cc1db42385bb6b1fe5bf2fac0ae08e520ecf43c5 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 8 Aug 2022 21:05:45 +0530 Subject: [PATCH 23/47] final refactor --- .../contentcuration/tests/test_change_signals.py | 8 ++++---- .../contentcuration/{ => tests}/utils/websocket_helper.py | 2 +- .../contentcuration/viewsets/websockets/signals.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) rename contentcuration/contentcuration/{ => tests}/utils/websocket_helper.py (99%) diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index 5c7ea7618a..ead96afc87 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -5,11 +5,11 @@ from contentcuration.models import Change from contentcuration.tests import testdata from contentcuration.tests.base import BucketTestMixin +from contentcuration.tests.utils.websocket_helper import create_channel_specific_change_object +from contentcuration.tests.utils.websocket_helper import create_channel_user_common_change_object +from contentcuration.tests.utils.websocket_helper import create_errored_change_object +from contentcuration.tests.utils.websocket_helper import create_user_specific_change_object from contentcuration.tests.viewsets.base import generate_update_event -from contentcuration.utils.websocket_helper import create_channel_specific_change_object -from contentcuration.utils.websocket_helper import create_channel_user_common_change_object -from contentcuration.utils.websocket_helper import create_errored_change_object -from contentcuration.utils.websocket_helper import create_user_specific_change_object from contentcuration.viewsets.sync.constants import CHANNEL diff --git a/contentcuration/contentcuration/utils/websocket_helper.py b/contentcuration/contentcuration/tests/utils/websocket_helper.py similarity index 99% rename from contentcuration/contentcuration/utils/websocket_helper.py rename to contentcuration/contentcuration/tests/utils/websocket_helper.py index 485c64cd4f..1f1f67eefb 100644 --- a/contentcuration/contentcuration/utils/websocket_helper.py +++ b/contentcuration/contentcuration/tests/utils/websocket_helper.py @@ -25,7 +25,7 @@ def __init__(self, instance): "The change object did not have a created_by_id {}" ) self.message = message.format( - instance.__dict__ + instance.pk ) super(NoneCreatedByIdError, self).__init__(self.message) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index 43c81bfdaf..e7f0dd5179 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -6,8 +6,8 @@ from django.dispatch import receiver from contentcuration.models import Change +from contentcuration.tests.utils.websocket_helper import NoneCreatedByIdError from contentcuration.utils.sentry import report_exception -from contentcuration.utils.websocket_helper import NoneCreatedByIdError logging = logger.getLogger(__name__) @@ -26,7 +26,7 @@ def broadcast_new_change_model(instance): # name of indiviual_user group indiviual_room_group_name = instance.user_id - if(instance.created_by_id is None): + if instance.created_by_id is None: try: raise NoneCreatedByIdError(instance) except NoneCreatedByIdError as e: From d9c9b68076309bc697aad96cc9a2cea39e7237b6 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 8 Aug 2022 21:54:05 +0530 Subject: [PATCH 24/47] moved NoneCreatedByIdError --- .../tests/utils/websocket_helper.py | 19 ------------------ .../viewsets/websockets/signals.py | 20 ++++++++++++++++++- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/contentcuration/contentcuration/tests/utils/websocket_helper.py b/contentcuration/contentcuration/tests/utils/websocket_helper.py index 1f1f67eefb..a87b816ab3 100644 --- a/contentcuration/contentcuration/tests/utils/websocket_helper.py +++ b/contentcuration/contentcuration/tests/utils/websocket_helper.py @@ -12,25 +12,6 @@ from contentcuration.viewsets.sync.constants import EDITOR_M2M -class NoneCreatedByIdError(Exception): - """ - Use to log change object whose created_by_id is set to none. We don't raise this error, - just feed it to Sentry for reporting. - """ - - def __init__(self, instance): - - self.change_object = instance - message = ( - "The change object did not have a created_by_id {}" - ) - self.message = message.format( - instance.pk - ) - - super(NoneCreatedByIdError, self).__init__(self.message) - - def bookmark_metadata(channel): return { "channel": channel.id, diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index e7f0dd5179..a5087f35df 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -6,12 +6,30 @@ from django.dispatch import receiver from contentcuration.models import Change -from contentcuration.tests.utils.websocket_helper import NoneCreatedByIdError from contentcuration.utils.sentry import report_exception logging = logger.getLogger(__name__) +class NoneCreatedByIdError(Exception): + """ + Use to log change object whose created_by_id is set to none. We don't raise this error, + just feed it to Sentry for reporting. + """ + + def __init__(self, instance): + + self.change_object = instance + message = ( + "The change object did not have a created_by_id {}" + ) + self.message = message.format( + instance.pk + ) + + super(NoneCreatedByIdError, self).__init__(self.message) + + @receiver(post_save, sender=Change, weak=False) def broadcast_new_change_model_handler(sender, instance, created, **kwargs): broadcast_new_change_model(instance) From 035455e0f73632624d08e556747a1000c1cc3711 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Tue, 9 Aug 2022 07:41:26 -0700 Subject: [PATCH 25/47] Update websocket consumer to use new task implementation --- .../contentcuration/viewsets/websockets/consumers.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 1a159818c5..07b1583ea5 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -6,7 +6,8 @@ from contentcuration.models import Change from contentcuration.models import Channel -from contentcuration.tasks import get_or_create_async_task +from contentcuration.tasks import apply_channel_changes_task +from contentcuration.tasks import apply_user_changes_task from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.constants import CREATED @@ -119,9 +120,9 @@ def receive(self, text_data): disallowed_changes.append(c) change_models = Change.create_changes(user_only_changes + channel_changes, created_by_id=user_id, session_key=session_key) if user_only_changes: - get_or_create_async_task("apply_user_changes", self.user, user_id=user_id) + apply_user_changes_task.fetch_or_enqueue(self.user, user_id=user_id) for channel_id in allowed_ids: - get_or_create_async_task("apply_channel_changes", self.user, channel_id=channel_id) + apply_channel_changes_task.fetch_or_enqueue(self.user, channel_id=channel_id) allowed_changes = [{"rev": c.client_rev, "server_rev": c.server_rev} for c in change_models] response_payload.update({"disallowed": disallowed_changes, "allowed": allowed_changes}) From c571454ab3cea7aa0b01937bfe2512afae51c5ed Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Wed, 17 Aug 2022 09:06:41 -0700 Subject: [PATCH 26/47] Remove imports not caught in merge conflict resolution --- .../contentcuration/frontend/shared/data/serverSync.js | 1 - 1 file changed, 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 57a2b26c5b..2ee296a325 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -13,7 +13,6 @@ import { MAX_REV_KEY, } from './constants'; import { Channel, Session, Task } from './resources'; -import { cleanupLocks, hasActiveLocks } from './changes'; import { INDEXEDDB_RESOURCES } from './registry'; import applyChanges from './applyRemoteChanges'; From bed4154dc008cf300d27f1585c494f9d137c0810 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 22 Aug 2022 22:31:14 +0530 Subject: [PATCH 27/47] remould frontend to remove fake task object --- .../frontend/shared/data/serverSync.js | 20 ++-- .../tests/test_change_signals.py | 2 +- .../contentcuration/viewsets/base.py | 92 ++++++++++++++++--- .../contentcuration/viewsets/sync/endpoint.py | 20 ---- .../viewsets/websockets/consumers.py | 22 +++++ .../viewsets/websockets/signals.py | 11 ++- 6 files changed, 118 insertions(+), 49 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 2ee296a325..7072b07fe2 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -4,6 +4,11 @@ import get from 'lodash/get'; import orderBy from 'lodash/orderBy'; import pick from 'lodash/pick'; import uniq from 'lodash/uniq'; +import mergeAllChanges from './mergeChanges'; +import db from './db'; +import applyChanges from './applyRemoteChanges'; +import { INDEXEDDB_RESOURCES } from './registry'; +import { Channel, Session, Task } from './resources'; import { ACTIVE_CHANNELS, CHANGES_TABLE, @@ -12,12 +17,6 @@ import { IGNORED_SOURCE, MAX_REV_KEY, } from './constants'; -import { Channel, Session, Task } from './resources'; - -import { INDEXEDDB_RESOURCES } from './registry'; -import applyChanges from './applyRemoteChanges'; -import db from './db'; -import mergeAllChanges from './mergeChanges'; import client from 'shared/client'; import urls from 'shared/urls'; @@ -201,11 +200,6 @@ function handleMaxRevs(response, userId) { return Promise.all(promises); } -function handleTasks(response) { - const tasks = get(response, ['data', 'tasks'], []); - return Task.setTasks(tasks); -} - async function syncChanges() { // Note: we could in theory use Dexie syncable for what // we are doing here, but I can't find a good way to make @@ -284,6 +278,9 @@ async function syncChanges() { socket.onmessage = function(e) { const data = JSON.parse(e.data); console.log(data); + if (data.task) { + Task.put(data.task); + } }; try { @@ -294,7 +291,6 @@ async function syncChanges() { handleErrors(response), handleSuccesses(response), handleMaxRevs(response, user.id), - handleTasks(response), ]); } catch (err) { console.error('There was an error updating change status: ', err); // eslint-disable-line no-console diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index ead96afc87..f3bfcf8c47 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -102,6 +102,6 @@ def test_signal_handler_errored_changes(self, mock_get_channel_layer, mock_async async_mock_return_value = mock_async_to_sync.return_value assert 1 == mock_async_to_sync.call_count async_mock_return_value.assert_called_once_with(str(self.user.id), { - 'type': 'broadcast_changes', + 'type': 'broadcast_errors', 'errored': change_serialized }) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index f2e786ab07..16facb21c6 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -3,7 +3,9 @@ import uuid from contextlib import contextmanager +from asgiref.sync import async_to_sync from celery import states +from channels.layers import get_channel_layer from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q from django.http import Http404 @@ -894,24 +896,70 @@ def create_change_tracker(pk, table, channel_id, user, task_name): # Clean up any previous tasks specific to this in case there were failures. meta = json.dumps(dict(pk=pk, table=table)) TaskResult.objects.filter(channel_id=channel_id, task_name=task_name, meta=meta).delete() + progress = 0 + status = states.STARTED + + channel_layer = get_channel_layer() + room_group_name = channel_id task_id = uuid.uuid4().hex - task_object = TaskResult.objects.create( - task_id=task_id, - status=states.STARTED, - channel_id=channel_id, - task_name=task_name, - user=user, - meta=meta + + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': None, + 'progress': progress, + 'channel_id': channel_id, + 'status': status, + } + } ) def update_progress(progress=None): if progress: - task_object.progress = progress - task_object.save() + if progress == 100: + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': None, + 'progress': progress, + 'channel_id': channel_id, + 'status': states.SUCCESS, + } + } + ) + else: + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': None, + 'progress': progress, + 'channel_id': channel_id, + 'status': status, + } + } + ) Change.create_change( - generate_update_event(pk, table, {TASK_ID: task_object.task_id}, channel_id=channel_id), applied=True + generate_update_event(pk, table, {TASK_ID: task_id}, channel_id=channel_id), applied=True ) tracker = ProgressTracker(task_id, update_progress) @@ -919,13 +967,27 @@ def update_progress(progress=None): try: yield tracker except Exception as e: - task_object.status = states.FAILURE - task_object.traceback = e.__traceback__ - task_object.save() + status = states.FAILURE + traceback_str = ''.join(traceback.format_tb(e.__traceback__)) + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': traceback_str, + 'progress': progress, + 'channel_id': channel_id, + 'status': status, + } + } + ) finally: - if task_object.status == states.STARTED: + if status == states.SUCCESS: # No error reported, cleanup. Change.create_change( generate_update_event(pk, table, {TASK_ID: None}, channel_id=channel_id), applied=True ) - task_object.delete() diff --git a/contentcuration/contentcuration/viewsets/sync/endpoint.py b/contentcuration/contentcuration/viewsets/sync/endpoint.py index 17cb6dca68..be57184954 100644 --- a/contentcuration/contentcuration/viewsets/sync/endpoint.py +++ b/contentcuration/contentcuration/viewsets/sync/endpoint.py @@ -3,7 +3,6 @@ and deals with processing all the changes to make appropriate bulk creates, updates, and deletes. """ -from celery import states from django.db.models import Q from rest_framework.authentication import SessionAuthentication from rest_framework.permissions import IsAuthenticated @@ -12,7 +11,6 @@ from contentcuration.models import Change from contentcuration.models import Channel -from contentcuration.models import TaskResult from contentcuration.tasks import apply_channel_changes_task from contentcuration.tasks import apply_user_changes_task from contentcuration.viewsets.sync.constants import CHANNEL @@ -119,22 +117,6 @@ def return_changes(self, request, channel_revs): return {"changes": changes, "errors": errors, "successes": successes} - def return_tasks(self, request, channel_revs): - tasks = TaskResult.objects.filter( - channel_id__in=channel_revs.keys(), - status__in=[states.STARTED, states.FAILURE], - ).exclude(task_name__in=[apply_channel_changes_task.name, apply_user_changes_task.name]) - return { - "tasks": tasks.values( - "task_id", - "task_name", - "traceback", - "progress", - "channel_id", - "status", - ) - } - def post(self, request): response_payload = { "disallowed": [], @@ -151,6 +133,4 @@ def post(self, request): response_payload.update(self.return_changes(request, channel_revs)) - response_payload.update(self.return_tasks(request, channel_revs)) - return Response(response_payload) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 07b1583ea5..e426a85e81 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -140,3 +140,25 @@ def broadcast_changes(self, event): self.send(text_data=json.dumps({ 'change': change })) + + def broadcast_errors(self, event): + """ + Broadcast any errors to frontend + """ + error = event['errored'] + + # Send message to WebSocket + self.send(text_data=json.dumps({ + 'errored': error + })) + + def broadcast_tasks(self, event): + """ + Broadcast tasks to frontend + """ + task = event['tasks'] + + # Send message to WebSocket + self.send(text_data=json.dumps({ + 'task': task + })) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index a5087f35df..8b41c84f86 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -44,6 +44,15 @@ def broadcast_new_change_model(instance): # name of indiviual_user group indiviual_room_group_name = instance.user_id + if instance.created_by_id is None and instance.change_type == 2 and instance.errored is True: + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_errors', + 'errored': serialized_change_object + } + ) + if instance.created_by_id is None: try: raise NoneCreatedByIdError(instance) @@ -57,7 +66,7 @@ def broadcast_new_change_model(instance): async_to_sync(channel_layer.group_send)( str(instance.created_by_id), { - 'type': 'broadcast_changes', + 'type': 'broadcast_errors', 'errored': serialized_change_object } ) From 80f0a87e30b45f9123f0fe8794f4752073686e8b Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 22 Aug 2022 23:26:16 +0530 Subject: [PATCH 28/47] Refactor create_change_tracker --- .../contentcuration/viewsets/base.py | 66 +++++++++---------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index 16facb21c6..2bbf655291 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -923,40 +923,22 @@ def create_change_tracker(pk, table, channel_id, user, task_name): def update_progress(progress=None): if progress: - if progress == 100: - async_to_sync(channel_layer.group_send)( - str(room_group_name), - { - 'type': 'broadcast_tasks', - 'tasks': { - 'pk': pk, - 'table': table, - 'task_id': task_id, - 'task_name': task_name, - 'traceback': None, - 'progress': progress, - 'channel_id': channel_id, - 'status': states.SUCCESS, - } - } - ) - else: - async_to_sync(channel_layer.group_send)( - str(room_group_name), - { - 'type': 'broadcast_tasks', - 'tasks': { - 'pk': pk, - 'table': table, - 'task_id': task_id, - 'task_name': task_name, - 'traceback': None, - 'progress': progress, - 'channel_id': channel_id, - 'status': status, - } + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': None, + 'progress': progress, + 'channel_id': channel_id, + 'status': status, } - ) + } + ) Change.create_change( generate_update_event(pk, table, {TASK_ID: task_id}, channel_id=channel_id), applied=True @@ -986,7 +968,23 @@ def update_progress(progress=None): } ) finally: - if status == states.SUCCESS: + if status == states.STARTED: + async_to_sync(channel_layer.group_send)( + str(room_group_name), + { + 'type': 'broadcast_tasks', + 'tasks': { + 'pk': pk, + 'table': table, + 'task_id': task_id, + 'task_name': task_name, + 'traceback': None, + 'progress': progress, + 'channel_id': channel_id, + 'status': states.SUCCESS, + } + } + ) # No error reported, cleanup. Change.create_change( generate_update_event(pk, table, {TASK_ID: None}, channel_id=channel_id), applied=True From 15a401449d3f65706395c3d18a1588df35659f12 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Mon, 22 Aug 2022 23:30:02 +0530 Subject: [PATCH 29/47] fix progress after succes --- contentcuration/contentcuration/viewsets/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index 2bbf655291..13281ce3e8 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -969,6 +969,7 @@ def update_progress(progress=None): ) finally: if status == states.STARTED: + progress = 100 async_to_sync(channel_layer.group_send)( str(room_group_name), { From 49da0b7d8c1ba1b9667d99d691c9464e5b3cd243 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 31 Aug 2022 22:30:15 +0530 Subject: [PATCH 30/47] add recipe for daphne server and change nginx file --- Makefile | 15 ++++++++++++--- deploy/nginx.conf.jinja2 | 13 +++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 98fe7f78c6..845f496aae 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,17 @@ -altprodserver: NUM_PROCS:=3 -altprodserver: NUM_THREADS:=5 -altprodserver: collectstatic ensurecrowdinclient downloadmessages compilemessages +altprodserver: + $(MAKE) -j 2 gunicornanddapneserver + +gunicornanddapneserver: gunicornserver daphneserver + +daphneserver: + cd contentcuration/ && daphne -b 0.0.0.0 -p 8082 contentcuration.asgi:application + +gunicornserver: NUM_PROCS:=2 +gunicornserver: NUM_THREADS:=5 +gunicornserver: collectstatic ensurecrowdinclient downloadmessages compilemessages cd contentcuration/ && gunicorn contentcuration.wsgi:application --timeout=4000 --error-logfile=/var/log/gunicorn-error.log --workers=${NUM_PROCS} --threads=${NUM_THREADS} --bind=0.0.0.0:8081 --pid=/tmp/contentcuration.pid --log-level=debug || sleep infinity + contentnodegc: cd contentcuration/ && python manage.py garbage_collect diff --git a/deploy/nginx.conf.jinja2 b/deploy/nginx.conf.jinja2 index c762a077b3..890c7abeb6 100644 --- a/deploy/nginx.conf.jinja2 +++ b/deploy/nginx.conf.jinja2 @@ -24,6 +24,11 @@ http { server 127.0.0.1:8081; } + # Proxy to daphne for websocket requests + upstream ws_server { + server 127.0.0.1:8082; + } + # Allow-list filter for content catalog paths server { listen 8080; @@ -84,6 +89,14 @@ http { proxy_set_header Host $host; } + location /ws/ { + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection "upgrade"; + proxy_redirect off; + proxy_pass http://ws_server; + } + + location /content/ { proxy_http_version 1.1; proxy_pass {{ $aws_s3_endpoint_url }}/{{ $aws_s3_bucket_name }}/; From c4393729a9f1a05288c9ebeacb117a749af11f24 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 24 Aug 2022 20:08:11 +0530 Subject: [PATCH 31/47] change serverSync.js --- .../frontend/shared/data/serverSync.js | 44 ++++++++++--------- .../viewsets/websockets/consumers.py | 11 +++++ .../viewsets/websockets/signals.py | 15 +++++-- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 057b669050..f48587c35a 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -80,10 +80,9 @@ function trimChangeForSync(change) { } } -function handleDisallowed(response) { +function handleDisallowed(disallowed) { // The disallowed property is an array of any changes that were sent to the server, // that were rejected. - const disallowed = get(response, ['data', 'disallowed'], []); if (disallowed.length) { // Collect all disallowed const disallowedRevs = disallowed.map(d => Number(d.rev)); @@ -97,10 +96,9 @@ function handleDisallowed(response) { return Promise.resolve(); } -function handleAllowed(response) { +function handleAllowed(allowed) { // The allowed property is an array of any rev and server_rev for any changes sent to // the server that were accepted - const allowed = get(response, ['data', 'allowed'], []); if (allowed.length) { const revMap = {}; for (let obj of allowed) { @@ -116,21 +114,19 @@ function handleAllowed(response) { return Promise.resolve(); } -function handleReturnedChanges(response) { +function handleReturnedChanges(returnedChanges) { // The changes property is an array of any changes from the server to apply in the // client. - const returnedChanges = get(response, ['data', 'changes'], []); if (returnedChanges.length) { return applyChanges(returnedChanges); } return Promise.resolve(); } -function handleErrors(response) { +function handleErrors(errors) { // The errors property is an array of any changes that were sent to the server, // that were rejected, with an additional errors property that describes // the error. - const errors = get(response, ['data', 'errors'], []); if (errors.length) { const errorMap = {}; for (let error of errors) { @@ -148,13 +144,12 @@ function handleErrors(response) { return Promise.resolve(); } -function handleSuccesses(response) { +function handleSuccesses(success) { // The successes property is an array of server_revs for any previously synced changes // that have now been successfully applied on the server. - const successes = get(response, ['data', 'successes'], []); - if (successes.length) { + if (success) { return db[CHANGES_TABLE].where('server_rev') - .anyOf(successes.map(c => c.server_rev)) + .anyOf(success.server_rev) .delete(); } return Promise.resolve(); @@ -281,17 +276,26 @@ async function syncChanges() { if (data.task) { Task.put(data.task); } + if (data.responsePayload && data.responsePayload.allowed) { + handleAllowed(data.responsePayload.allowed); + } + if (data.responsePayload && data.responsePayload.disallowed) { + handleDisallowed(data.responsePayload.disallowed); + } + if (data.errored) { + handleErrors(data.errored); + } + if (data.success); + { + handleSuccesses(data.success); + } + if (data.change) { + handleReturnedChanges(data.change); + } }; try { - await Promise.all([ - handleDisallowed(response), - handleAllowed(response), - handleReturnedChanges(response), - handleErrors(response), - handleSuccesses(response), - handleMaxRevs(response, user.id), - ]); + await Promise.all([handleMaxRevs(response, user.id)]); } catch (err) { console.error('There was an error updating change status: ', err); // eslint-disable-line no-console } diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index e426a85e81..4efb3d1f24 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -162,3 +162,14 @@ def broadcast_tasks(self, event): self.send(text_data=json.dumps({ 'task': task })) + + def broadcast_success(self, event): + """ + Broadcast success to required user + """ + success = event['success'] + + # Send message to WebSocket + self.send(text_data=json.dumps({ + 'success': success + })) diff --git a/contentcuration/contentcuration/viewsets/websockets/signals.py b/contentcuration/contentcuration/viewsets/websockets/signals.py index 8b41c84f86..600fd27039 100644 --- a/contentcuration/contentcuration/viewsets/websockets/signals.py +++ b/contentcuration/contentcuration/viewsets/websockets/signals.py @@ -73,6 +73,13 @@ def broadcast_new_change_model(instance): if instance.applied: # if the change is related to channel we broadcast changes to channel group if not indiviual_room_group_name and room_group_name: + async_to_sync(channel_layer.group_send)( + str(instance.created_by_id), + { + 'type': 'broadcast_success', + 'success': serialized_change_object + } + ) async_to_sync(channel_layer.group_send)( str(room_group_name), { @@ -85,8 +92,8 @@ def broadcast_new_change_model(instance): async_to_sync(channel_layer.group_send)( str(indiviual_room_group_name), { - 'type': 'broadcast_changes', - 'change': serialized_change_object + 'type': 'broadcast_success', + 'success': serialized_change_object } ) # if the change is realted to both user and channel then we will broadcast to both of the groups @@ -101,7 +108,7 @@ def broadcast_new_change_model(instance): async_to_sync(channel_layer.group_send)( str(indiviual_room_group_name), { - 'type': 'broadcast_changes', - 'change': serialized_change_object + 'type': 'broadcast_success', + 'success': serialized_change_object } ) From 29e4a6617518182dcba0823f2c67d4e969466eee Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 25 Aug 2022 14:04:30 +0530 Subject: [PATCH 32/47] restructure websocket code --- .../frontend/shared/data/serverSync.js | 254 ++++++++++++++---- 1 file changed, 201 insertions(+), 53 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index f48587c35a..b245b93cf4 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -1,14 +1,3 @@ -import debounce from 'lodash/debounce'; -import findLastIndex from 'lodash/findLastIndex'; -import get from 'lodash/get'; -import orderBy from 'lodash/orderBy'; -import pick from 'lodash/pick'; -import uniq from 'lodash/uniq'; -import mergeAllChanges from './mergeChanges'; -import db from './db'; -import applyChanges from './applyRemoteChanges'; -import { INDEXEDDB_RESOURCES } from './registry'; -import { Channel, Session, Task } from './resources'; import { ACTIVE_CHANNELS, CHANGES_TABLE, @@ -17,7 +6,20 @@ import { IGNORED_SOURCE, MAX_REV_KEY, } from './constants'; +import { Channel, Session, Task } from './resources'; + +import { INDEXEDDB_RESOURCES } from './registry'; +import applyChanges from './applyRemoteChanges'; import client from 'shared/client'; +import db from './db'; +import debounce from 'lodash/debounce'; +// import { event } from 'jquery'; +import findLastIndex from 'lodash/findLastIndex'; +import get from 'lodash/get'; +import mergeAllChanges from './mergeChanges'; +import orderBy from 'lodash/orderBy'; +import pick from 'lodash/pick'; +import uniq from 'lodash/uniq'; import urls from 'shared/urls'; // When this many seconds pass without a syncable @@ -26,7 +28,7 @@ const SYNC_IF_NO_CHANGES_FOR = 2; // When this many seconds pass, repoll the backend to // check for any updates to active channels, or the user and sync any current changes -const SYNC_POLL_INTERVAL = 5; +// const SYNC_POLL_INTERVAL = 5; let socket; // Flag to check if a sync is currently active. let syncActive = false; @@ -80,9 +82,115 @@ function trimChangeForSync(change) { } } -function handleDisallowed(disallowed) { +function socketHandleAllowed(allowed) { + if (allowed.length) { + const revMap = {}; + for (let obj of allowed) { + revMap[obj.rev] = obj.server_rev; + } + return db[CHANGES_TABLE].where('rev') + .anyOf(Object.keys(revMap).map(Number)) + .modify(c => { + c.server_rev = revMap[c.rev]; + c.synced = true; + }); + } +} +function socketHandleDisallowed(disallowed) { + if (disallowed.length) { + // Collect all disallowed + const disallowedRevs = disallowed.map(d => Number(d.rev)); + // Set the return error data onto the changes - this will update the change + // both with any errors and the results of any merging that happened prior + // to the sync operation being called + return db[CHANGES_TABLE].where('rev') + .anyOf(disallowedRevs) + .modify({ disallowed: true, synced: true }); + } +} + +function socketHandleReturnedChanges(returnedChange) { + // The changes property is an array of any changes from the server to apply in the + // client. + let returnedChanges = []; + returnedChanges.push(returnedChange); + if (returnedChanges.length) { + return applyChanges(returnedChanges); + } +} + +function socketHandleErrors(error) { + // The errors property is an array of any changes that were sent to the server, + // that were rejected, with an additional errors property that describes + // the error. + let errors = []; + errors.push(error); + if (errors.length) { + const errorMap = {}; + for (let error of errors) { + errorMap[error.server_rev] = error; + } + // Set the return error data onto the changes - this will update the change + // both with any errors and the results of any merging that happened prior + // to the sync operation being called + return db[CHANGES_TABLE].where('server_rev') + .anyOf(Object.keys(errorMap)) + .modify(obj => { + return Object.assign(obj, errorMap[obj.server_rev]); + }); + } +} + +function socketHandleSuccesses(success) { + // The successes property is an array of server_revs for any previously synced changes + // that have now been successfully applied on the server. + if (success.length) { + return db[CHANGES_TABLE].where('server_rev') + .anyOf(success.server_rev) + .delete(); + } +} + +function socketHandleMaxRevs(change, userId) { + let changes = []; + changes.push(change); + const allChanges = orderBy(changes, 'server_rev', 'desc'); + const channelIds = uniq(allChanges.channel_id).filter(Boolean); + const maxRevs = {}; + const promises = []; + for (let channelId of channelIds) { + const channelChanges = allChanges.filter(c => c.channel_id === channelId); + maxRevs[`${MAX_REV_KEY}.${channelId}`] = channelChanges[0].server_rev; + const lastChannelEditIndex = findLastIndex( + channelChanges, + c => !c.errors && !c.user_id && c.created_by_id && c.type !== CHANGE_TYPES.PUBLISHED + ); + const lastPublishIndex = findLastIndex( + channelChanges, + c => !c.errors && !c.user_id && c.created_by_id && c.type === CHANGE_TYPES.PUBLISHED + ); + if (lastChannelEditIndex > lastPublishIndex) { + promises.push( + Channel.transaction({ mode: 'rw', source: IGNORED_SOURCE }, () => { + return Channel.table.update(channelId, { unpublished_changes: true }); + }) + ); + } + } + const lastUserChange = allChanges.find(c => c.user_id === userId); + if (lastUserChange) { + maxRevs.user_rev = lastUserChange.server_rev; + } + if (Object.keys(maxRevs).length) { + promises.push(Session.updateSession(maxRevs)); + } + return Promise.all(promises); +} + +function handleDisallowed(response) { // The disallowed property is an array of any changes that were sent to the server, // that were rejected. + const disallowed = get(response, ['data', 'disallowed'], []); if (disallowed.length) { // Collect all disallowed const disallowedRevs = disallowed.map(d => Number(d.rev)); @@ -96,9 +204,10 @@ function handleDisallowed(disallowed) { return Promise.resolve(); } -function handleAllowed(allowed) { +function handleAllowed(response) { // The allowed property is an array of any rev and server_rev for any changes sent to // the server that were accepted + const allowed = get(response, ['data', 'allowed'], []); if (allowed.length) { const revMap = {}; for (let obj of allowed) { @@ -114,19 +223,21 @@ function handleAllowed(allowed) { return Promise.resolve(); } -function handleReturnedChanges(returnedChanges) { +function handleReturnedChanges(response) { // The changes property is an array of any changes from the server to apply in the // client. + const returnedChanges = get(response, ['data', 'changes'], []); if (returnedChanges.length) { return applyChanges(returnedChanges); } return Promise.resolve(); } -function handleErrors(errors) { +function handleErrors(response) { // The errors property is an array of any changes that were sent to the server, // that were rejected, with an additional errors property that describes // the error. + const errors = get(response, ['data', 'errors'], []); if (errors.length) { const errorMap = {}; for (let error of errors) { @@ -144,12 +255,13 @@ function handleErrors(errors) { return Promise.resolve(); } -function handleSuccesses(success) { +function handleSuccesses(response) { // The successes property is an array of server_revs for any previously synced changes // that have now been successfully applied on the server. - if (success) { + const successes = get(response, ['data', 'successes'], []); + if (successes.length) { return db[CHANGES_TABLE].where('server_rev') - .anyOf(success.server_rev) + .anyOf(successes.map(c => c.server_rev)) .delete(); } return Promise.resolve(); @@ -194,7 +306,39 @@ function handleMaxRevs(response, userId) { } return Promise.all(promises); } +async function WebsocketSendChanges(changesToSync) { + const user = await Session.getSession(); + + if (!user) { + // If not logged in, nothing to do. + return; + } + + const requestPayload = { + changes: [], + }; + + // By the time we get here, our changesToSync Array should + // have every change we want to sync to the server, so we + // can now trim it down to only what is needed to transmit over the wire. + // TODO: remove moves when a delete change is present for an object, + // because a delete will wipe out the move. + const changes = changesToSync.map(trimChangeForSync); + // Create a promise for the sync - if there is nothing to sync just resolve immediately, + // in order to still call our change cleanup code. + if (changes.length) { + requestPayload.changes = changes; + } + + if (requestPayload.changes.length != 0) { + socket.send( + JSON.stringify({ + payload: requestPayload, + }) + ); + } +} async function syncChanges() { // Note: we could in theory use Dexie syncable for what // we are doing here, but I can't find a good way to make @@ -262,40 +406,18 @@ async function syncChanges() { // "errors": [], // "successes": [], // } - if (requestPayload.changes.length != 0) { - socket.send( - JSON.stringify({ - payload: requestPayload, - }) - ); - } + const response = await client.post(urls['sync'](), requestPayload); - socket.onmessage = function(e) { - const data = JSON.parse(e.data); - console.log(data); - if (data.task) { - Task.put(data.task); - } - if (data.responsePayload && data.responsePayload.allowed) { - handleAllowed(data.responsePayload.allowed); - } - if (data.responsePayload && data.responsePayload.disallowed) { - handleDisallowed(data.responsePayload.disallowed); - } - if (data.errored) { - handleErrors(data.errored); - } - if (data.success); - { - handleSuccesses(data.success); - } - if (data.change) { - handleReturnedChanges(data.change); - } - }; try { - await Promise.all([handleMaxRevs(response, user.id)]); + await Promise.all([ + handleDisallowed(response), + handleAllowed(response), + handleReturnedChanges(response), + handleErrors(response), + handleSuccesses(response), + handleMaxRevs(response, user.id), + ]); } catch (err) { console.error('There was an error updating change status: ', err); // eslint-disable-line no-console } @@ -347,8 +469,9 @@ async function handleChanges(changes) { // If we detect changes were written to the changes table // then we'll trigger sync - if (syncableChanges.length || newChangeTableEntries) { - debouncedSyncChanges(); + + if (newChangeTableEntries.length) { + WebsocketSendChanges(newChangeTableEntries); } } @@ -363,6 +486,7 @@ export function startSyncing() { window.location.href ); websocketUrl.protocol = window.location.protocol == 'https:' ? 'wss:' : 'ws:'; + socket = new WebSocket(websocketUrl); // Connection opened @@ -372,8 +496,32 @@ export function startSyncing() { debouncedSyncChanges(); + socket.addEventListener('message', e => { + const data = JSON.parse(e.data); + console.log(data); + if (data.task) { + Task.put(data.task); + } + if (data.responsePayload && data.responsePayload.allowed) { + socketHandleAllowed(data.responsePayload.allowed); + } + if (data.responsePayload && data.responsePayload.disallowed) { + socketHandleDisallowed(data.responsePayload.disallowed); + } + if (data.change) { + socketHandleReturnedChanges(data.change); + socketHandleMaxRevs(data.change, data.change.created_by_id); + } + if (data.errored) { + socketHandleErrors(data.errored); + } + if (data.success) { + socketHandleSuccesses(data.success); + } + }); + // Start the sync interval - intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); + // intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); db.on('changes', handleChanges); } From 15eaf377e394ad10f9094ff4819a39ee3fc367b7 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 31 Aug 2022 22:03:36 +0530 Subject: [PATCH 33/47] added error handeling for socket functions --- .../frontend/shared/data/resources.js | 54 +++++++++---------- .../frontend/shared/data/serverSync.js | 54 ++++++++++++++++--- 2 files changed, 71 insertions(+), 37 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 5686301574..9852366d2d 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1,5 +1,25 @@ +import { + ACTIVE_CHANNELS, + CHANGES_TABLE, + CHANGE_TYPES, + CHANNEL_SYNC_KEEP_ALIVE_INTERVAL, + COPYING_FLAG, + CURRENT_USER, + IGNORED_SOURCE, + MAX_REV_KEY, + RELATIVE_TREE_POSITIONS, + TABLE_NAMES, + TASK_ID, +} from './constants'; +import { API_RESOURCES, INDEXEDDB_RESOURCES } from './registry'; +import { NEW_OBJECT, fileErrors } from 'shared/constants'; +import applyChanges, { applyMods, collectChanges } from './applyRemoteChanges'; +import client, { paramsSerializer } from 'shared/client'; +import db, { CLIENTID, Collection, channelScope } from './db'; + import Dexie from 'dexie'; import Mutex from 'mutex-js'; +import { currentLanguage } from 'shared/i18n'; import findIndex from 'lodash/findIndex'; import flatMap from 'lodash/flatMap'; import isArray from 'lodash/isArray'; @@ -7,34 +27,14 @@ import isFunction from 'lodash/isFunction'; import isNumber from 'lodash/isNumber'; import isString from 'lodash/isString'; import matches from 'lodash/matches'; +import mergeAllChanges from './mergeChanges'; import overEvery from 'lodash/overEvery'; import pick from 'lodash/pick'; import sortBy from 'lodash/sortBy'; import uniq from 'lodash/uniq'; import uniqBy from 'lodash/uniqBy'; - -import { v4 as uuidv4 } from 'uuid'; -import { - CHANGE_TYPES, - CHANGES_TABLE, - IGNORED_SOURCE, - RELATIVE_TREE_POSITIONS, - TABLE_NAMES, - COPYING_FLAG, - TASK_ID, - CURRENT_USER, - ACTIVE_CHANNELS, - CHANNEL_SYNC_KEEP_ALIVE_INTERVAL, - MAX_REV_KEY, -} from './constants'; -import applyChanges, { applyMods, collectChanges } from './applyRemoteChanges'; -import mergeAllChanges from './mergeChanges'; -import db, { channelScope, CLIENTID, Collection } from './db'; -import { API_RESOURCES, INDEXEDDB_RESOURCES } from './registry'; -import { fileErrors, NEW_OBJECT } from 'shared/constants'; -import client, { paramsSerializer } from 'shared/client'; -import { currentLanguage } from 'shared/i18n'; import urls from 'shared/urls'; +import { v4 as uuidv4 } from 'uuid'; // Number of seconds after which data is considered stale. const REFRESH_INTERVAL = 5; @@ -1808,15 +1808,9 @@ export const Clipboard = new TreeResource({ export const Task = new IndexedDBResource({ tableName: TABLE_NAMES.TASK, idField: 'task_id', - setTasks(tasks) { + setTask(task) { return this.transaction({ mode: 'rw', source: IGNORED_SOURCE }, () => { - return this.table - .where(this.idField) - .noneOf(tasks.map(t => t[this.idField])) - .delete() - .then(() => { - return this.table.bulkPut(tasks); - }); + return this.table.Put(task); }); }, }); diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index b245b93cf4..e7f9df6617 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -95,6 +95,7 @@ function socketHandleAllowed(allowed) { c.synced = true; }); } + return Promise.resolve(); } function socketHandleDisallowed(disallowed) { if (disallowed.length) { @@ -107,6 +108,7 @@ function socketHandleDisallowed(disallowed) { .anyOf(disallowedRevs) .modify({ disallowed: true, synced: true }); } + return Promise.resolve(); } function socketHandleReturnedChanges(returnedChange) { @@ -117,6 +119,7 @@ function socketHandleReturnedChanges(returnedChange) { if (returnedChanges.length) { return applyChanges(returnedChanges); } + return Promise.resolve(); } function socketHandleErrors(error) { @@ -139,6 +142,7 @@ function socketHandleErrors(error) { return Object.assign(obj, errorMap[obj.server_rev]); }); } + return Promise.resolve(); } function socketHandleSuccesses(success) { @@ -149,6 +153,7 @@ function socketHandleSuccesses(success) { .anyOf(success.server_rev) .delete(); } + return Promise.resolve(); } function socketHandleMaxRevs(change, userId) { @@ -255,6 +260,11 @@ function handleErrors(response) { return Promise.resolve(); } +function handleTasks(response) { + const tasks = get(response, ['data', 'tasks'], []); + return Task.setTasks(tasks); +} + function handleSuccesses(response) { // The successes property is an array of server_revs for any previously synced changes // that have now been successfully applied on the server. @@ -306,9 +316,11 @@ function handleMaxRevs(response, userId) { } return Promise.all(promises); } + async function WebsocketSendChanges(changesToSync) { const user = await Session.getSession(); + if (!user) { // If not logged in, nothing to do. return; @@ -417,6 +429,7 @@ async function syncChanges() { handleErrors(response), handleSuccesses(response), handleMaxRevs(response, user.id), + handleTasks(response), ]); } catch (err) { console.error('There was an error updating change status: ', err); // eslint-disable-line no-console @@ -494,29 +507,56 @@ export function startSyncing() { console.log('Websocket connected'); }); + // Listen for any errors due to which connection may be closed. + socket.addEventListener('error', event => { + console.log('WebSocket error: ', event); + }); + debouncedSyncChanges(); socket.addEventListener('message', e => { const data = JSON.parse(e.data); console.log(data); if (data.task) { - Task.put(data.task); + Task.setTask(data.task); } if (data.responsePayload && data.responsePayload.allowed) { - socketHandleAllowed(data.responsePayload.allowed); + try { + socketHandleAllowed(data.responsePayload.allowed); + } catch (err) { + console.log(err); + } } if (data.responsePayload && data.responsePayload.disallowed) { - socketHandleDisallowed(data.responsePayload.disallowed); + try { + socketHandleDisallowed(data.responsePayload.disallowed); + } catch (err) { + console.log(err); + } } if (data.change) { - socketHandleReturnedChanges(data.change); - socketHandleMaxRevs(data.change, data.change.created_by_id); + try { + Promise.all([ + socketHandleReturnedChanges(data.change), + socketHandleMaxRevs(data.change, data.change.created_by_id), + ]); + } catch (err) { + console.log(err); + } } if (data.errored) { - socketHandleErrors(data.errored); + try { + socketHandleErrors(data.errored); + } catch (err) { + console.log(err); + } } if (data.success) { - socketHandleSuccesses(data.success); + try { + socketHandleSuccesses(data.success); + } catch (err) { + console.log(err); + } } }); From aed085d97dcd27ad4674a821b9b867866e9be2e9 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Fri, 2 Sep 2022 21:22:08 +0530 Subject: [PATCH 34/47] coHack --- .../frontend/shared/data/resources.js | 47 +++++++++---------- .../frontend/shared/data/serverSync.js | 31 ++++++------ 2 files changed, 37 insertions(+), 41 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 9852366d2d..95596eb8cd 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1,25 +1,5 @@ -import { - ACTIVE_CHANNELS, - CHANGES_TABLE, - CHANGE_TYPES, - CHANNEL_SYNC_KEEP_ALIVE_INTERVAL, - COPYING_FLAG, - CURRENT_USER, - IGNORED_SOURCE, - MAX_REV_KEY, - RELATIVE_TREE_POSITIONS, - TABLE_NAMES, - TASK_ID, -} from './constants'; -import { API_RESOURCES, INDEXEDDB_RESOURCES } from './registry'; -import { NEW_OBJECT, fileErrors } from 'shared/constants'; -import applyChanges, { applyMods, collectChanges } from './applyRemoteChanges'; -import client, { paramsSerializer } from 'shared/client'; -import db, { CLIENTID, Collection, channelScope } from './db'; - import Dexie from 'dexie'; import Mutex from 'mutex-js'; -import { currentLanguage } from 'shared/i18n'; import findIndex from 'lodash/findIndex'; import flatMap from 'lodash/flatMap'; import isArray from 'lodash/isArray'; @@ -27,14 +7,33 @@ import isFunction from 'lodash/isFunction'; import isNumber from 'lodash/isNumber'; import isString from 'lodash/isString'; import matches from 'lodash/matches'; -import mergeAllChanges from './mergeChanges'; import overEvery from 'lodash/overEvery'; import pick from 'lodash/pick'; import sortBy from 'lodash/sortBy'; import uniq from 'lodash/uniq'; import uniqBy from 'lodash/uniqBy'; -import urls from 'shared/urls'; import { v4 as uuidv4 } from 'uuid'; +import mergeAllChanges from './mergeChanges'; +import db, { CLIENTID, Collection, channelScope } from './db'; +import applyChanges, { applyMods, collectChanges } from './applyRemoteChanges'; +import { API_RESOURCES, INDEXEDDB_RESOURCES } from './registry'; +import { + ACTIVE_CHANNELS, + CHANGES_TABLE, + CHANGE_TYPES, + CHANNEL_SYNC_KEEP_ALIVE_INTERVAL, + COPYING_FLAG, + CURRENT_USER, + IGNORED_SOURCE, + MAX_REV_KEY, + RELATIVE_TREE_POSITIONS, + TABLE_NAMES, + TASK_ID, +} from './constants'; +import urls from 'shared/urls'; +import { currentLanguage } from 'shared/i18n'; +import client, { paramsSerializer } from 'shared/client'; +import { NEW_OBJECT, fileErrors } from 'shared/constants'; // Number of seconds after which data is considered stale. const REFRESH_INTERVAL = 5; @@ -1808,9 +1807,9 @@ export const Clipboard = new TreeResource({ export const Task = new IndexedDBResource({ tableName: TABLE_NAMES.TASK, idField: 'task_id', - setTask(task) { + setTasks(task) { return this.transaction({ mode: 'rw', source: IGNORED_SOURCE }, () => { - return this.table.Put(task); + return this.table.bulkPut(task); }); }, }); diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index e7f9df6617..c7de4e4389 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -1,3 +1,15 @@ +import debounce from 'lodash/debounce'; +// import { event } from 'jquery'; +import findLastIndex from 'lodash/findLastIndex'; +import get from 'lodash/get'; +import orderBy from 'lodash/orderBy'; +import pick from 'lodash/pick'; +import uniq from 'lodash/uniq'; +import mergeAllChanges from './mergeChanges'; +import db from './db'; +import applyChanges from './applyRemoteChanges'; +import { INDEXEDDB_RESOURCES } from './registry'; +import { Channel, Session, Task } from './resources'; import { ACTIVE_CHANNELS, CHANGES_TABLE, @@ -6,20 +18,7 @@ import { IGNORED_SOURCE, MAX_REV_KEY, } from './constants'; -import { Channel, Session, Task } from './resources'; - -import { INDEXEDDB_RESOURCES } from './registry'; -import applyChanges from './applyRemoteChanges'; import client from 'shared/client'; -import db from './db'; -import debounce from 'lodash/debounce'; -// import { event } from 'jquery'; -import findLastIndex from 'lodash/findLastIndex'; -import get from 'lodash/get'; -import mergeAllChanges from './mergeChanges'; -import orderBy from 'lodash/orderBy'; -import pick from 'lodash/pick'; -import uniq from 'lodash/uniq'; import urls from 'shared/urls'; // When this many seconds pass without a syncable @@ -318,8 +317,7 @@ function handleMaxRevs(response, userId) { } async function WebsocketSendChanges(changesToSync) { - const user = await Session.getSession(); - + const user = await Session.getSession(); if (!user) { // If not logged in, nothing to do. @@ -342,7 +340,6 @@ async function WebsocketSendChanges(changesToSync) { requestPayload.changes = changes; } - if (requestPayload.changes.length != 0) { socket.send( JSON.stringify({ @@ -518,7 +515,7 @@ export function startSyncing() { const data = JSON.parse(e.data); console.log(data); if (data.task) { - Task.setTask(data.task); + Task.setTasks([data.task]); } if (data.responsePayload && data.responsePayload.allowed) { try { From 9ce67071a011ee3fc3f3ad5baa799f36f9ceddba Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Sat, 3 Sep 2022 01:43:10 +0530 Subject: [PATCH 35/47] Strict created_by_id to not be none --- contentcuration/contentcuration/asgi.py | 3 +++ .../frontend/shared/data/serverSync.js | 4 ++-- contentcuration/contentcuration/settings.py | 2 +- .../contentcuration/tests/test_change_signals.py | 2 +- contentcuration/contentcuration/viewsets/base.py | 13 ++++++++++--- contentcuration/contentcuration/viewsets/channel.py | 4 ++-- .../viewsets/websockets/consumers.py | 8 ++++---- 7 files changed, 23 insertions(+), 13 deletions(-) diff --git a/contentcuration/contentcuration/asgi.py b/contentcuration/contentcuration/asgi.py index 4973a69aef..0ce54ecb18 100644 --- a/contentcuration/contentcuration/asgi.py +++ b/contentcuration/contentcuration/asgi.py @@ -1,9 +1,12 @@ +import django from channels.auth import AuthMiddlewareStack from channels.routing import ProtocolTypeRouter from channels.routing import URLRouter from contentcuration.viewsets.websockets.routing import websocket_urlpatterns +django.setup(set_prefix=False) + application = ProtocolTypeRouter({ "websocket": AuthMiddlewareStack( diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index c7de4e4389..e3232c2f22 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -459,7 +459,7 @@ async function handleChanges(changes) { // MOVE, COPY, PUBLISH, and SYNC changes where we may be executing them inside an IGNORED_SOURCE // because they also make UPDATE and CREATE changes that we wish to make in the client only. // Only do this for changes that are not marked as synced. - const newChangeTableEntries = changes.some( + const newChangeTableEntries = changes.filter( c => c.table === CHANGES_TABLE && c.type === CHANGE_TYPES.CREATED && !c.obj.synced ); @@ -481,7 +481,7 @@ async function handleChanges(changes) { // then we'll trigger sync if (newChangeTableEntries.length) { - WebsocketSendChanges(newChangeTableEntries); + WebsocketSendChanges(newChangeTableEntries.map(c => c.obj)); } } diff --git a/contentcuration/contentcuration/settings.py b/contentcuration/contentcuration/settings.py index 9c4a845ac8..cc93bbc9e5 100644 --- a/contentcuration/contentcuration/settings.py +++ b/contentcuration/contentcuration/settings.py @@ -66,7 +66,6 @@ # Application definition INSTALLED_APPS = ( - 'channels', 'contentcuration.apps.ContentConfig', 'django.contrib.auth', 'django.contrib.contenttypes', @@ -87,6 +86,7 @@ 'django_filters', 'mathfilters', 'django_celery_results', + 'channels', ) SESSION_ENGINE = "django.contrib.sessions.backends.cached_db" diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index f3bfcf8c47..caab1859ff 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -35,7 +35,7 @@ def test_change_signal_handler(self, mock_signal): """ self.client.force_login(self.user) new_name = "This is not the old name" - Change.create_change(generate_update_event(self.channel.id, CHANNEL, {"name": new_name}, channel_id=self.channel.id)) + Change.create_change(generate_update_event(self.channel.id, CHANNEL, {"name": new_name}, channel_id=self.channel.id), created_by_id=self.user.id) assert mock_signal.call_count == 1 @patch('contentcuration.viewsets.websockets.signals.async_to_sync') diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index 13281ce3e8..4967398730 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -14,6 +14,7 @@ from django_filters.constants import EMPTY_VALUES from django_filters.rest_framework import DjangoFilterBackend from django_filters.rest_framework import FilterSet +from rest_framework.exceptions import NotAuthenticated from rest_framework.filters import OrderingFilter from rest_framework.generics import get_object_or_404 from rest_framework.response import Response @@ -201,8 +202,13 @@ def create(self, validated_data): def save(self, **kwargs): instance = super(BulkModelSerializer, self).save(**kwargs) + if "request" in self.context and not self.context["request"].user.is_anonymous: + user = self.context["request"].user.id + else: + raise NotAuthenticated() + if self.changes: - Change.create_changes(self.changes, applied=True) + Change.create_changes(self.changes, applied=True, created_by_id=user) return instance @@ -941,7 +947,7 @@ def update_progress(progress=None): ) Change.create_change( - generate_update_event(pk, table, {TASK_ID: task_id}, channel_id=channel_id), applied=True + generate_update_event(pk, table, {TASK_ID: task_id}, channel_id=channel_id), applied=True, created_by_id=user.id ) tracker = ProgressTracker(task_id, update_progress) @@ -988,5 +994,6 @@ def update_progress(progress=None): ) # No error reported, cleanup. Change.create_change( - generate_update_event(pk, table, {TASK_ID: None}, channel_id=channel_id), applied=True + generate_update_event(pk, table, {TASK_ID: None}, channel_id=channel_id), applied=True, created_by_id=user.id + ) diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 517e66f0fe..237917d60d 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -490,13 +490,13 @@ def publish(self, pk, version_notes="", language=None): "unpublished_changes": _unpublished_changes_query(channel.id).exists() }, channel_id=channel.id ), - ], applied=True) + ], created_by_id=self.request.user.id, applied=True) except Exception: Change.create_changes([ generate_update_event( channel.id, CHANNEL, {"publishing": False, "unpublished_changes": True}, channel_id=channel.id ), - ], applied=True) + ], created_by_id=self.request.user, applied=True) raise def sync_from_changes(self, changes): diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 4efb3d1f24..6ab93064a8 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -4,10 +4,6 @@ from asgiref.sync import async_to_sync from channels.generic.websocket import WebsocketConsumer -from contentcuration.models import Change -from contentcuration.models import Channel -from contentcuration.tasks import apply_channel_changes_task -from contentcuration.tasks import apply_user_changes_task from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.constants import CREATED @@ -80,6 +76,10 @@ def disconnect(self, close_code): ) def receive(self, text_data): + from contentcuration.models import Change + from contentcuration.models import Channel + from contentcuration.tasks import apply_channel_changes_task + from contentcuration.tasks import apply_user_changes_task """ Executes when data is received from websocket """ From 262360b78e5c986e3e687d499ff8f98e8a338213 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 7 Sep 2022 01:16:10 +0530 Subject: [PATCH 36/47] refactored serverSync.js --- .../shared/data/applyRemoteChanges.js | 11 +++++++++- .../frontend/shared/data/resources.js | 19 +++++++++++++++-- .../frontend/shared/data/serverSync.js | 4 +--- contentcuration/contentcuration/models.py | 5 ++--- .../tests/test_change_signals.py | 21 ++++++++++++------- .../contentcuration/tests/test_serializers.py | 10 +++++++-- .../contentcuration/viewsets/base.py | 4 ++-- .../contentcuration/viewsets/channel.py | 11 +++++----- .../viewsets/websockets/consumers.py | 7 ++++--- 9 files changed, 62 insertions(+), 30 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js index 2400915f96..89cc5059f8 100644 --- a/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js +++ b/contentcuration/contentcuration/frontend/shared/data/applyRemoteChanges.js @@ -2,9 +2,9 @@ import Dexie from 'dexie'; import flatten from 'lodash/flatten'; import sortBy from 'lodash/sortBy'; import uniq from 'lodash/uniq'; -import { CHANGE_TYPES, IGNORED_SOURCE, TABLE_NAMES } from './constants'; import db from './db'; import { INDEXEDDB_RESOURCES } from './registry'; +import { CHANGE_TYPES, IGNORED_SOURCE, TABLE_NAMES } from './constants'; const { CREATED, DELETED, UPDATED, MOVED, PUBLISHED, SYNCED } = CHANGE_TYPES; @@ -91,6 +91,15 @@ export default function applyChanges(changes) { changes = sortBy(changes, ['server_rev', 'rev']); const table_names = uniq(changes.map(c => c.table)); + // When changes include a publish change + //add the contentnode table since `applyPublish` above needs to + // make updates to content nodes + if ( + changes.some(c => c.type === CHANGE_TYPES.PUBLISHED) && + !table_names.includes(TABLE_NAMES.CONTENTNODE) + ) { + table_names.push(TABLE_NAMES.CONTENTNODE); + } const tables = table_names.map(table => db.table(table)); return db.transaction('rw', tables, () => { diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 95596eb8cd..525ebe4427 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -2,6 +2,7 @@ import Dexie from 'dexie'; import Mutex from 'mutex-js'; import findIndex from 'lodash/findIndex'; import flatMap from 'lodash/flatMap'; +import intersection from 'lodash/intersection'; import isArray from 'lodash/isArray'; import isFunction from 'lodash/isFunction'; import isNumber from 'lodash/isNumber'; @@ -35,6 +36,12 @@ import { currentLanguage } from 'shared/i18n'; import client, { paramsSerializer } from 'shared/client'; import { NEW_OBJECT, fileErrors } from 'shared/constants'; +/** + * Task names for which it is only useful to keep the most recent task object + * @type {string[]} + */ +const SINGULAR_TASKS = ['export-channel', 'sync-channel']; + // Number of seconds after which data is considered stale. const REFRESH_INTERVAL = 5; @@ -1807,9 +1814,17 @@ export const Clipboard = new TreeResource({ export const Task = new IndexedDBResource({ tableName: TABLE_NAMES.TASK, idField: 'task_id', - setTasks(task) { + setTasks(tasks) { return this.transaction({ mode: 'rw', source: IGNORED_SOURCE }, () => { - return this.table.bulkPut(task); + let deletePromise = Promise.resolve(); + let taskDeletes = intersection( + SINGULAR_TASKS, + tasks.map(t => t.task_name) + ); + if (taskDeletes.length) { + deletePromise = this.table.filter(t => taskDeletes.includes(t.task_name)).delete(); + } + return deletePromise.then(() => this.table.bulkPut(tasks)); }); }, }); diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index e3232c2f22..9d2043815e 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -490,7 +490,7 @@ let intervalTimer; export function startSyncing() { // Initiate a sync immediately in case any data // is left over in the database. - + debouncedSyncChanges(); const websocketUrl = new URL( `/ws/sync_socket/${window.CHANNEL_EDIT_GLOBAL.channel_id}/`, window.location.href @@ -509,8 +509,6 @@ export function startSyncing() { console.log('WebSocket error: ', event); }); - debouncedSyncChanges(); - socket.addEventListener('message', e => { const data = JSON.parse(e.data); console.log(data); diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index cc68b58855..fa27bbd925 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -8,7 +8,6 @@ from datetime import datetime import pytz -from celery import states from django.conf import settings from django.contrib.auth.base_user import AbstractBaseUser from django.contrib.auth.base_user import BaseUserManager @@ -2414,7 +2413,7 @@ def _create_from_change(cls, created_by_id=None, channel_id=None, user_id=None, ) @classmethod - def create_changes(cls, changes, created_by_id=None, session_key=None, applied=False): + def create_changes(cls, changes, created_by_id, session_key=None, applied=False): change_models = [] for change in changes: change_models.append(cls._create_from_change(created_by_id=created_by_id, session_key=session_key, applied=applied, **change)) @@ -2423,7 +2422,7 @@ def create_changes(cls, changes, created_by_id=None, session_key=None, applied=F return change_models @classmethod - def create_change(cls, change, created_by_id=None, session_key=None, applied=False): + def create_change(cls, change, created_by_id, session_key=None, applied=False): obj = cls._create_from_change(created_by_id=created_by_id, session_key=session_key, applied=applied, **change) obj.save() return obj diff --git a/contentcuration/contentcuration/tests/test_change_signals.py b/contentcuration/contentcuration/tests/test_change_signals.py index caab1859ff..5182330803 100644 --- a/contentcuration/contentcuration/tests/test_change_signals.py +++ b/contentcuration/contentcuration/tests/test_change_signals.py @@ -46,9 +46,14 @@ def test_signal_handler_channel_specific_changes(self, mock_get_channel_layer, m """ change_serialized = create_channel_specific_change_object(self.user, self.channel) channel_layer = mock_get_channel_layer.return_value - mock_async_to_sync.assert_called_once_with(channel_layer.group_send) + assert 2 == mock_async_to_sync.call_count + mock_async_to_sync.assert_called_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_once_with(self.channel.id, { + async_mock_return_value.assert_any_call(str(self.user.id), { + 'type': 'broadcast_success', + 'success': change_serialized + }) + async_mock_return_value.assert_any_call(self.channel.id, { 'type': 'broadcast_changes', 'change': change_serialized }) @@ -63,9 +68,9 @@ def test_signal_handler_user_specific_changes(self, mock_get_channel_layer, mock channel_layer = mock_get_channel_layer.return_value mock_async_to_sync.assert_called_once_with(channel_layer.group_send) async_mock_return_value = mock_async_to_sync.return_value - async_mock_return_value.assert_called_once_with(str(self.user.id), { - 'type': 'broadcast_changes', - 'change': change_serialized + async_mock_return_value.assert_any_call(str(self.user.id), { + 'type': 'broadcast_success', + 'success': change_serialized }) @patch('contentcuration.viewsets.websockets.signals.async_to_sync') @@ -85,9 +90,9 @@ def test_signal_handler_user_channel_common_changes(self, mock_get_channel_layer 'type': 'broadcast_changes', 'change': change_serialized }) - async_mock_return_value.assert_any_call(str(editor.id), { - 'type': 'broadcast_changes', - 'change': change_serialized + async_mock_return_value.assert_any_call(str(self.user.id), { + 'type': 'broadcast_success', + 'success': change_serialized }) @patch('contentcuration.viewsets.websockets.signals.async_to_sync') diff --git a/contentcuration/contentcuration/tests/test_serializers.py b/contentcuration/contentcuration/tests/test_serializers.py index 25331a7daf..029947ca74 100644 --- a/contentcuration/contentcuration/tests/test_serializers.py +++ b/contentcuration/contentcuration/tests/test_serializers.py @@ -148,12 +148,15 @@ class Meta: nested_writes = True def test_save__create(self): + class request: + user = self.user + context = {"request": request} s = self.ChannelSerializer( data=dict( name="New test channel", description="This is the best test channel", content_defaults=dict(author="Buster"), - ) + ), context=context ) self.assertTrue(s.is_valid()) @@ -164,6 +167,9 @@ def test_save__create(self): self.assertEqual(defaults, c.content_defaults) def test_save__update(self): + class request: + user = self.user + context = {"request": request} c = Channel( name="New test channel", description="This is the best test channel", @@ -172,7 +178,7 @@ def test_save__update(self): c.save() s = self.ChannelSerializer( - c, data=dict(content_defaults=dict(license="Special Permissions")) + c, data=dict(content_defaults=dict(license="Special Permissions")), context=context ) self.assertTrue(s.is_valid()) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index 4967398730..d93e056bcf 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -203,12 +203,12 @@ def create(self, validated_data): def save(self, **kwargs): instance = super(BulkModelSerializer, self).save(**kwargs) if "request" in self.context and not self.context["request"].user.is_anonymous: - user = self.context["request"].user.id + user = self.context["request"].user else: raise NotAuthenticated() if self.changes: - Change.create_changes(self.changes, applied=True, created_by_id=user) + Change.create_changes(self.changes, applied=True, created_by_id=user.id) return instance diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 237917d60d..bf3fa7d909 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -480,23 +480,22 @@ def publish(self, pk, version_notes="", language=None): progress_tracker=progress_tracker, language=language ) - Change.create_changes([ + Change.create_change( generate_update_event( channel.id, CHANNEL, { "published": True, "publishing": False, "primary_token": channel.get_human_token().token, - "last_published": channel.last_published, - "unpublished_changes": _unpublished_changes_query(channel.id).exists() + "last_published": str(channel.last_published), + "unpublished_changes": bool(_unpublished_changes_query(channel.id).exists()) }, channel_id=channel.id - ), - ], created_by_id=self.request.user.id, applied=True) + ), created_by_id=self.request.user.id, applied=True) except Exception: Change.create_changes([ generate_update_event( channel.id, CHANNEL, {"publishing": False, "unpublished_changes": True}, channel_id=channel.id ), - ], created_by_id=self.request.user, applied=True) + ], created_by_id=self.request.user.id, applied=True) raise def sync_from_changes(self, changes): diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 6ab93064a8..68a87ff379 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -76,13 +76,14 @@ def disconnect(self, close_code): ) def receive(self, text_data): + """ + Executes when data is received from websocket + """ from contentcuration.models import Change from contentcuration.models import Channel from contentcuration.tasks import apply_channel_changes_task from contentcuration.tasks import apply_user_changes_task - """ - Executes when data is received from websocket - """ + response_payload = { "disallowed": [], "allowed": [], From 55e3be894272e8980cc007f7dd98146cf8d3e77b Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 8 Sep 2022 10:37:12 +0530 Subject: [PATCH 37/47] remove unnecessary comments --- .../frontend/shared/data/serverSync.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 9d2043815e..d8c77fe64f 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -1,5 +1,4 @@ import debounce from 'lodash/debounce'; -// import { event } from 'jquery'; import findLastIndex from 'lodash/findLastIndex'; import get from 'lodash/get'; import orderBy from 'lodash/orderBy'; @@ -25,9 +24,6 @@ import urls from 'shared/urls'; // change being registered, sync changes! const SYNC_IF_NO_CHANGES_FOR = 2; -// When this many seconds pass, repoll the backend to -// check for any updates to active channels, or the user and sync any current changes -// const SYNC_POLL_INTERVAL = 5; let socket; // Flag to check if a sync is currently active. let syncActive = false; @@ -515,16 +511,16 @@ export function startSyncing() { if (data.task) { Task.setTasks([data.task]); } - if (data.responsePayload && data.responsePayload.allowed) { + if (data.response_payload && data.response_payload.allowed) { try { - socketHandleAllowed(data.responsePayload.allowed); + socketHandleAllowed(data.response_payload.allowed); } catch (err) { console.log(err); } } - if (data.responsePayload && data.responsePayload.disallowed) { + if (data.response_payload && data.response_payload.disallowed) { try { - socketHandleDisallowed(data.responsePayload.disallowed); + socketHandleDisallowed(data.response_payload.disallowed); } catch (err) { console.log(err); } From f9874ec55d63e8ff3a16fd2147eb44a19db2d9ec Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 8 Sep 2022 11:18:09 +0530 Subject: [PATCH 38/47] refactor serverSync.js --- .../frontend/shared/data/serverSync.js | 243 ++++++------------ 1 file changed, 77 insertions(+), 166 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index d8c77fe64f..fa8a0de47e 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -77,120 +77,9 @@ function trimChangeForSync(change) { } } -function socketHandleAllowed(allowed) { - if (allowed.length) { - const revMap = {}; - for (let obj of allowed) { - revMap[obj.rev] = obj.server_rev; - } - return db[CHANGES_TABLE].where('rev') - .anyOf(Object.keys(revMap).map(Number)) - .modify(c => { - c.server_rev = revMap[c.rev]; - c.synced = true; - }); - } - return Promise.resolve(); -} -function socketHandleDisallowed(disallowed) { - if (disallowed.length) { - // Collect all disallowed - const disallowedRevs = disallowed.map(d => Number(d.rev)); - // Set the return error data onto the changes - this will update the change - // both with any errors and the results of any merging that happened prior - // to the sync operation being called - return db[CHANGES_TABLE].where('rev') - .anyOf(disallowedRevs) - .modify({ disallowed: true, synced: true }); - } - return Promise.resolve(); -} - -function socketHandleReturnedChanges(returnedChange) { - // The changes property is an array of any changes from the server to apply in the - // client. - let returnedChanges = []; - returnedChanges.push(returnedChange); - if (returnedChanges.length) { - return applyChanges(returnedChanges); - } - return Promise.resolve(); -} - -function socketHandleErrors(error) { - // The errors property is an array of any changes that were sent to the server, - // that were rejected, with an additional errors property that describes - // the error. - let errors = []; - errors.push(error); - if (errors.length) { - const errorMap = {}; - for (let error of errors) { - errorMap[error.server_rev] = error; - } - // Set the return error data onto the changes - this will update the change - // both with any errors and the results of any merging that happened prior - // to the sync operation being called - return db[CHANGES_TABLE].where('server_rev') - .anyOf(Object.keys(errorMap)) - .modify(obj => { - return Object.assign(obj, errorMap[obj.server_rev]); - }); - } - return Promise.resolve(); -} - -function socketHandleSuccesses(success) { - // The successes property is an array of server_revs for any previously synced changes - // that have now been successfully applied on the server. - if (success.length) { - return db[CHANGES_TABLE].where('server_rev') - .anyOf(success.server_rev) - .delete(); - } - return Promise.resolve(); -} - -function socketHandleMaxRevs(change, userId) { - let changes = []; - changes.push(change); - const allChanges = orderBy(changes, 'server_rev', 'desc'); - const channelIds = uniq(allChanges.channel_id).filter(Boolean); - const maxRevs = {}; - const promises = []; - for (let channelId of channelIds) { - const channelChanges = allChanges.filter(c => c.channel_id === channelId); - maxRevs[`${MAX_REV_KEY}.${channelId}`] = channelChanges[0].server_rev; - const lastChannelEditIndex = findLastIndex( - channelChanges, - c => !c.errors && !c.user_id && c.created_by_id && c.type !== CHANGE_TYPES.PUBLISHED - ); - const lastPublishIndex = findLastIndex( - channelChanges, - c => !c.errors && !c.user_id && c.created_by_id && c.type === CHANGE_TYPES.PUBLISHED - ); - if (lastChannelEditIndex > lastPublishIndex) { - promises.push( - Channel.transaction({ mode: 'rw', source: IGNORED_SOURCE }, () => { - return Channel.table.update(channelId, { unpublished_changes: true }); - }) - ); - } - } - const lastUserChange = allChanges.find(c => c.user_id === userId); - if (lastUserChange) { - maxRevs.user_rev = lastUserChange.server_rev; - } - if (Object.keys(maxRevs).length) { - promises.push(Session.updateSession(maxRevs)); - } - return Promise.all(promises); -} - -function handleDisallowed(response) { +function handleDisallowed(disallowed) { // The disallowed property is an array of any changes that were sent to the server, // that were rejected. - const disallowed = get(response, ['data', 'disallowed'], []); if (disallowed.length) { // Collect all disallowed const disallowedRevs = disallowed.map(d => Number(d.rev)); @@ -204,10 +93,9 @@ function handleDisallowed(response) { return Promise.resolve(); } -function handleAllowed(response) { +function handleAllowed(allowed) { // The allowed property is an array of any rev and server_rev for any changes sent to // the server that were accepted - const allowed = get(response, ['data', 'allowed'], []); if (allowed.length) { const revMap = {}; for (let obj of allowed) { @@ -223,21 +111,19 @@ function handleAllowed(response) { return Promise.resolve(); } -function handleReturnedChanges(response) { +function handleReturnedChanges(returnedChanges) { // The changes property is an array of any changes from the server to apply in the // client. - const returnedChanges = get(response, ['data', 'changes'], []); if (returnedChanges.length) { return applyChanges(returnedChanges); } return Promise.resolve(); } -function handleErrors(response) { +function handleErrors(errors) { // The errors property is an array of any changes that were sent to the server, // that were rejected, with an additional errors property that describes // the error. - const errors = get(response, ['data', 'errors'], []); if (errors.length) { const errorMap = {}; for (let error of errors) { @@ -255,15 +141,13 @@ function handleErrors(response) { return Promise.resolve(); } -function handleTasks(response) { - const tasks = get(response, ['data', 'tasks'], []); +function handleTasks(tasks) { return Task.setTasks(tasks); } -function handleSuccesses(response) { +function handleSuccesses(successes) { // The successes property is an array of server_revs for any previously synced changes // that have now been successfully applied on the server. - const successes = get(response, ['data', 'successes'], []); if (successes.length) { return db[CHANGES_TABLE].where('server_rev') .anyOf(successes.map(c => c.server_rev)) @@ -272,14 +156,8 @@ function handleSuccesses(response) { return Promise.resolve(); } -function handleMaxRevs(response, userId) { - const allChanges = orderBy( - get(response, ['data', 'changes'], []) - .concat(get(response, ['data', 'errors'], [])) - .concat(get(response, ['data', 'successes'], [])), - 'server_rev', - 'desc' - ); +function handleMaxRevs(changes, userId) { + const allChanges = orderBy(changes, 'server_rev', 'desc'); const channelIds = uniq(allChanges.map(c => c.channel_id)).filter(Boolean); const maxRevs = {}; const promises = []; @@ -312,37 +190,70 @@ function handleMaxRevs(response, userId) { return Promise.all(promises); } -async function WebsocketSendChanges(changesToSync) { - const user = await Session.getSession(); +async function WebsocketSendChanges() { + // Note: we could in theory use Dexie syncable for what + // we are doing here, but I can't find a good way to make + // it ignore our regular API calls for seeding the database + // Also, the pattern it expects for server interactions would + // require greater backend rearchitecting to focus our server-client + // interactions on changes to objects, with consistent and resolvable + // revisions. We will do this for now, but we have the option of doing + // something more involved and better architectured in the future. + + syncActive = true; + // Track the maxRevision at this moment so that we can ignore any changes that + // might have come in during processing - leave them for the next cycle. + // This is the primary key of the change objects, so the collection is ordered by this + // by default - if we just grab the last object, we can get the key from there. + const [lastChange, user] = await Promise.all([ + db[CHANGES_TABLE].orderBy('rev').last(), + Session.getSession(), + ]); if (!user) { // If not logged in, nothing to do. return; } + const now = Date.now(); + const channelIds = Object.entries(user[ACTIVE_CHANNELS] || {}) + .filter(([id, time]) => id && time > now - CHANNEL_SYNC_KEEP_ALIVE_INTERVAL) + .map(([id]) => id); + const channel_revs = {}; + for (let channelId of channelIds) { + channel_revs[channelId] = get(user, [MAX_REV_KEY, channelId], 0); + } + const requestPayload = { changes: [], + channel_revs, + user_rev: user.user_rev || 0, }; - // By the time we get here, our changesToSync Array should - // have every change we want to sync to the server, so we - // can now trim it down to only what is needed to transmit over the wire. - // TODO: remove moves when a delete change is present for an object, - // because a delete will wipe out the move. - const changes = changesToSync.map(trimChangeForSync); - // Create a promise for the sync - if there is nothing to sync just resolve immediately, - // in order to still call our change cleanup code. - if (changes.length) { - requestPayload.changes = changes; - } - - if (requestPayload.changes.length != 0) { - socket.send( - JSON.stringify({ - payload: requestPayload, - }) - ); + if (lastChange) { + const changesMaxRevision = lastChange.rev; + const syncableChanges = db[CHANGES_TABLE].where('rev') + .belowOrEqual(changesMaxRevision) + .filter(c => !c.synced); + const changesToSync = await syncableChanges.toArray(); + // By the time we get here, our changesToSync Array should + // have every change we want to sync to the server, so we + // can now trim it down to only what is needed to transmit over the wire. + // TODO: remove moves when a delete change is present for an object, + // because a delete will wipe out the move. + const changes = changesToSync.map(trimChangeForSync); + // Create a promise for the sync - if there is nothing to sync just resolve immediately, + // in order to still call our change cleanup code. + if (changes.length) { + requestPayload.changes = changes; + socket.send( + JSON.stringify({ + payload: requestPayload, + }) + ); + } } + syncActive = false; } async function syncChanges() { // Note: we could in theory use Dexie syncable for what @@ -416,13 +327,13 @@ async function syncChanges() { try { await Promise.all([ - handleDisallowed(response), - handleAllowed(response), - handleReturnedChanges(response), - handleErrors(response), - handleSuccesses(response), - handleMaxRevs(response, user.id), - handleTasks(response), + handleDisallowed(get(response, ['data', 'disallowed'], [])), + handleAllowed(get(response, ['data', 'allowed'], [])), + handleReturnedChanges(get(response, ['data', 'changes'], [])), + handleErrors(get(response, ['data', 'errors'], [])), + handleSuccesses(get(response, ['data', 'successes'], [])), + handleMaxRevs(get(response, ['data', 'changes'], []), user.id), + handleTasks(get(response, ['data', 'tasks'], [])), ]); } catch (err) { console.error('There was an error updating change status: ', err); // eslint-disable-line no-console @@ -477,7 +388,9 @@ async function handleChanges(changes) { // then we'll trigger sync if (newChangeTableEntries.length) { - WebsocketSendChanges(newChangeTableEntries.map(c => c.obj)); + if (!syncActive) { + WebsocketSendChanges(); + } } } @@ -513,14 +426,14 @@ export function startSyncing() { } if (data.response_payload && data.response_payload.allowed) { try { - socketHandleAllowed(data.response_payload.allowed); + handleAllowed(data.response_payload.allowed); } catch (err) { console.log(err); } } if (data.response_payload && data.response_payload.disallowed) { try { - socketHandleDisallowed(data.response_payload.disallowed); + handleDisallowed(data.response_payload.disallowed); } catch (err) { console.log(err); } @@ -528,8 +441,8 @@ export function startSyncing() { if (data.change) { try { Promise.all([ - socketHandleReturnedChanges(data.change), - socketHandleMaxRevs(data.change, data.change.created_by_id), + handleReturnedChanges([data.change]), + handleMaxRevs([data.change], data.change.created_by_id), ]); } catch (err) { console.log(err); @@ -537,22 +450,20 @@ export function startSyncing() { } if (data.errored) { try { - socketHandleErrors(data.errored); + handleErrors([data.errored]); } catch (err) { console.log(err); } } if (data.success) { try { - socketHandleSuccesses(data.success); + handleSuccesses([data.success]); } catch (err) { console.log(err); } } }); - // Start the sync interval - // intervalTimer = setInterval(debouncedSyncChanges, SYNC_POLL_INTERVAL * 1000); db.on('changes', handleChanges); } From d83df35a08f4658031bb5b63fd4252ed782bd9dc Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 8 Sep 2022 12:05:59 +0530 Subject: [PATCH 39/47] change debounce time --- .../contentcuration/frontend/shared/data/serverSync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index fa8a0de47e..8179ea2846 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -22,7 +22,7 @@ import urls from 'shared/urls'; // When this many seconds pass without a syncable // change being registered, sync changes! -const SYNC_IF_NO_CHANGES_FOR = 2; +const SYNC_IF_NO_CHANGES_FOR = 0.5; let socket; // Flag to check if a sync is currently active. From d970e2dcaabb60da9d48e43dc7b0b74acb93d5bc Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 8 Sep 2022 22:23:56 +0530 Subject: [PATCH 40/47] small tweaks --- .../frontend/shared/data/serverSync.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 8179ea2846..6fbaa0c6d2 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -332,7 +332,12 @@ async function syncChanges() { handleReturnedChanges(get(response, ['data', 'changes'], [])), handleErrors(get(response, ['data', 'errors'], [])), handleSuccesses(get(response, ['data', 'successes'], [])), - handleMaxRevs(get(response, ['data', 'changes'], []), user.id), + handleMaxRevs( + get(response, ['data', 'changes'], []) + .concat(get(response, ['data', 'errors'], [])) + .concat(get(response, ['data', 'successes'], [])), + user.id + ), handleTasks(get(response, ['data', 'tasks'], [])), ]); } catch (err) { @@ -440,10 +445,8 @@ export function startSyncing() { } if (data.change) { try { - Promise.all([ - handleReturnedChanges([data.change]), - handleMaxRevs([data.change], data.change.created_by_id), - ]); + handleReturnedChanges([data.change]); + handleMaxRevs([data.change], data.change.created_by_id); } catch (err) { console.log(err); } @@ -451,6 +454,7 @@ export function startSyncing() { if (data.errored) { try { handleErrors([data.errored]); + handleMaxRevs([data.errored], data.errored.created_by_id); } catch (err) { console.log(err); } @@ -458,6 +462,7 @@ export function startSyncing() { if (data.success) { try { handleSuccesses([data.success]); + handleMaxRevs([data.success], data.success.created_by_id); } catch (err) { console.log(err); } From dd4c54139419242bd020e804461165c54ee8857d Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Thu, 8 Sep 2022 23:14:17 +0530 Subject: [PATCH 41/47] change created_by_id in MaxRev to user.id --- .../contentcuration/frontend/shared/data/serverSync.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/serverSync.js b/contentcuration/contentcuration/frontend/shared/data/serverSync.js index 6fbaa0c6d2..531c25dc84 100644 --- a/contentcuration/contentcuration/frontend/shared/data/serverSync.js +++ b/contentcuration/contentcuration/frontend/shared/data/serverSync.js @@ -423,8 +423,9 @@ export function startSyncing() { console.log('WebSocket error: ', event); }); - socket.addEventListener('message', e => { + socket.addEventListener('message', async e => { const data = JSON.parse(e.data); + const user = await Session.getSession(); console.log(data); if (data.task) { Task.setTasks([data.task]); @@ -446,7 +447,7 @@ export function startSyncing() { if (data.change) { try { handleReturnedChanges([data.change]); - handleMaxRevs([data.change], data.change.created_by_id); + handleMaxRevs([data.change], user.id); } catch (err) { console.log(err); } @@ -454,7 +455,7 @@ export function startSyncing() { if (data.errored) { try { handleErrors([data.errored]); - handleMaxRevs([data.errored], data.errored.created_by_id); + handleMaxRevs([data.errored], user.id); } catch (err) { console.log(err); } @@ -462,7 +463,7 @@ export function startSyncing() { if (data.success) { try { handleSuccesses([data.success]); - handleMaxRevs([data.success], data.success.created_by_id); + handleMaxRevs([data.success], user.id); } catch (err) { console.log(err); } From 5cbd4ce24fe0b706ba79a07c7f92a8b9ce0fe9a7 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Thu, 8 Sep 2022 11:37:50 -0700 Subject: [PATCH 42/47] Fix bad merging of Makefile changes --- Makefile | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 57018fc4c7..501ee380c2 100644 --- a/Makefile +++ b/Makefile @@ -17,17 +17,13 @@ daphneserver: gunicornserver: NUM_PROCS:=2 gunicornserver: NUM_THREADS:=5 -gunicornserver: collectstatic ensurecrowdinclient downloadmessages compilemessages +gunicornserver: collectstatic compilemessages cd contentcuration/ && gunicorn contentcuration.wsgi:application --timeout=4000 --error-logfile=/var/log/gunicorn-error.log --workers=${NUM_PROCS} --threads=${NUM_THREADS} --bind=0.0.0.0:8081 --pid=/tmp/contentcuration.pid --log-level=debug || sleep infinity contentnodegc: cd contentcuration/ && python manage.py garbage_collect -dummyusers: - cd contentcuration/ && python manage.py loaddata contentcuration/fixtures/admin_user.json - cd contentcuration/ && python manage.py loaddata contentcuration/fixtures/admin_user_token.json - prodceleryworkers: cd contentcuration/ && celery -A contentcuration worker -l info --concurrency=3 --task-events @@ -41,9 +37,6 @@ migrate: python contentcuration/manage.py migrate || true python contentcuration/manage.py loadconstants -contentnodegc: - python contentcuration/manage.py garbage_collect - filedurations: python contentcuration/manage.py set_file_duration From 64e4fd70b61efe61b766ba39940fc6e7c5a49f44 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Thu, 8 Sep 2022 14:35:33 -0700 Subject: [PATCH 43/47] Remove all make dependencies since they weren't being run anyway --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 501ee380c2..bb430dd669 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ daphneserver: gunicornserver: NUM_PROCS:=2 gunicornserver: NUM_THREADS:=5 -gunicornserver: collectstatic compilemessages +gunicornserver: cd contentcuration/ && gunicorn contentcuration.wsgi:application --timeout=4000 --error-logfile=/var/log/gunicorn-error.log --workers=${NUM_PROCS} --threads=${NUM_THREADS} --bind=0.0.0.0:8081 --pid=/tmp/contentcuration.pid --log-level=debug || sleep infinity From 56707136af779c59b85cff97c56f19f1f6a14c3d Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Thu, 8 Sep 2022 14:49:12 -0700 Subject: [PATCH 44/47] Reduce procs to better match current production (2), to avoid overloading --- Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index bb430dd669..5d3c35cf71 100644 --- a/Makefile +++ b/Makefile @@ -15,10 +15,9 @@ gunicornanddapneserver: gunicornserver daphneserver daphneserver: cd contentcuration/ && daphne -b 0.0.0.0 -p 8082 contentcuration.asgi:application -gunicornserver: NUM_PROCS:=2 -gunicornserver: NUM_THREADS:=5 +gunicornserver: NUM_PROCS:=1 gunicornserver: - cd contentcuration/ && gunicorn contentcuration.wsgi:application --timeout=4000 --error-logfile=/var/log/gunicorn-error.log --workers=${NUM_PROCS} --threads=${NUM_THREADS} --bind=0.0.0.0:8081 --pid=/tmp/contentcuration.pid --log-level=debug || sleep infinity + cd contentcuration/ && gunicorn contentcuration.wsgi:application --timeout=4000 --error-logfile=/var/log/gunicorn-error.log --workers=${NUM_PROCS} --bind=0.0.0.0:8081 --pid=/tmp/contentcuration.pid --log-level=debug || sleep infinity contentnodegc: From 1a4be5f2f78f59059e191d404b9b2a53f050e6b2 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Fri, 9 Sep 2022 11:51:21 -0700 Subject: [PATCH 45/47] Fix redis connection trying localhost in production --- contentcuration/contentcuration/settings.py | 4 ++-- deploy/nginx.conf.jinja2 | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/contentcuration/contentcuration/settings.py b/contentcuration/contentcuration/settings.py index bdae87ab37..99a15858d0 100644 --- a/contentcuration/contentcuration/settings.py +++ b/contentcuration/contentcuration/settings.py @@ -204,12 +204,12 @@ WSGI_APPLICATION = 'contentcuration.wsgi.application' ASGI_APPLICATION = 'contentcuration.asgi.application' - +CHANNELS_DB = 2 CHANNEL_LAYERS = { 'default': { 'BACKEND': 'channels_redis.core.RedisChannelLayer', 'CONFIG': { - "hosts": [('127.0.0.1', 6379)], + "hosts": ["{url}{db}".format(url=REDIS_URL, db=CHANNELS_DB)], }, }, } diff --git a/deploy/nginx.conf.jinja2 b/deploy/nginx.conf.jinja2 index 890c7abeb6..8c10268884 100644 --- a/deploy/nginx.conf.jinja2 +++ b/deploy/nginx.conf.jinja2 @@ -90,10 +90,12 @@ http { } location /ws/ { + proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; proxy_redirect off; proxy_pass http://ws_server; + gzip off; } From 21886346d81f12bc7ad94233148bfcb48d754202 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 12 Oct 2022 01:37:09 +0530 Subject: [PATCH 46/47] Added Health Check route for daphne --- contentcuration/contentcuration/asgi.py | 24 ++++++++++++++++--- .../viewsets/websockets/consumers.py | 9 +++++++ .../viewsets/websockets/routing.py | 4 ++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/asgi.py b/contentcuration/contentcuration/asgi.py index 0ce54ecb18..fcd1de6b9b 100644 --- a/contentcuration/contentcuration/asgi.py +++ b/contentcuration/contentcuration/asgi.py @@ -2,16 +2,34 @@ from channels.auth import AuthMiddlewareStack from channels.routing import ProtocolTypeRouter from channels.routing import URLRouter +from django.conf import settings +from contentcuration.viewsets.websockets.routing import http_urlpatterns from contentcuration.viewsets.websockets.routing import websocket_urlpatterns django.setup(set_prefix=False) -application = ProtocolTypeRouter({ - "websocket": +if settings.DEBUG: + # Settings for development environment + application = ProtocolTypeRouter({ + "websocket": AuthMiddlewareStack( URLRouter( websocket_urlpatterns ) ), -}) + }) +else: + # Settings for production environment + # we dont want to expose other url routes to daphne server + application = ProtocolTypeRouter({ + "http": + URLRouter(http_urlpatterns), + + "websocket": + AuthMiddlewareStack( + URLRouter( + websocket_urlpatterns + ) + ), + }) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 68a87ff379..78c72930f4 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -2,6 +2,7 @@ import logging as logger from asgiref.sync import async_to_sync +from channels.generic.http import AsyncHttpConsumer from channels.generic.websocket import WebsocketConsumer from contentcuration.viewsets.sync.constants import CHANNEL @@ -174,3 +175,11 @@ def broadcast_success(self, event): self.send(text_data=json.dumps({ 'success': success })) + +# Consumer for handeling the only http request related with +# Health check + + +class HealthCheckHttpConsumer(AsyncHttpConsumer): + async def handle(self, body): + await self.send_response(200, b"OK") diff --git a/contentcuration/contentcuration/viewsets/websockets/routing.py b/contentcuration/contentcuration/viewsets/websockets/routing.py index c334df7b03..2f2b894478 100644 --- a/contentcuration/contentcuration/viewsets/websockets/routing.py +++ b/contentcuration/contentcuration/viewsets/websockets/routing.py @@ -5,3 +5,7 @@ websocket_urlpatterns = [ re_path(r'ws/sync_socket/(?P\w+)/$', consumers.SyncConsumer.as_asgi()), ] + +http_urlpatterns = [ + re_path(r'healthz$', consumers.HealthCheckHttpConsumer.as_asgi()), +] From 0db9935725a2bd5742871667f32810e66bde21d1 Mon Sep 17 00:00:00 2001 From: Prathamesh Desai Date: Wed, 12 Oct 2022 02:48:00 +0530 Subject: [PATCH 47/47] removed repetition of code --- contentcuration/contentcuration/asgi.py | 26 ++++++------------- .../viewsets/websockets/consumers.py | 6 ++--- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/contentcuration/contentcuration/asgi.py b/contentcuration/contentcuration/asgi.py index fcd1de6b9b..8a63bc229a 100644 --- a/contentcuration/contentcuration/asgi.py +++ b/contentcuration/contentcuration/asgi.py @@ -9,27 +9,17 @@ django.setup(set_prefix=False) -if settings.DEBUG: - # Settings for development environment - application = ProtocolTypeRouter({ - "websocket": +protocol_config = { + "websocket": AuthMiddlewareStack( URLRouter( websocket_urlpatterns ) ), - }) -else: - # Settings for production environment - # we dont want to expose other url routes to daphne server - application = ProtocolTypeRouter({ - "http": - URLRouter(http_urlpatterns), +} - "websocket": - AuthMiddlewareStack( - URLRouter( - websocket_urlpatterns - ) - ), - }) +# production settings to add healthcheck +if not settings.DEBUG: + protocol_config.update(http=URLRouter(http_urlpatterns)) + +application = ProtocolTypeRouter(protocol_config) diff --git a/contentcuration/contentcuration/viewsets/websockets/consumers.py b/contentcuration/contentcuration/viewsets/websockets/consumers.py index 78c72930f4..722281be12 100644 --- a/contentcuration/contentcuration/viewsets/websockets/consumers.py +++ b/contentcuration/contentcuration/viewsets/websockets/consumers.py @@ -176,10 +176,10 @@ def broadcast_success(self, event): 'success': success })) -# Consumer for handeling the only http request related with -# Health check - class HealthCheckHttpConsumer(AsyncHttpConsumer): + """ + Consumer for handeling the only http request related with Health check + """ async def handle(self, body): await self.send_response(200, b"OK")