From 307a1cb029fb3f31ba19b555bc57b4655f6378af Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Mon, 5 Oct 2020 22:49:35 +0600 Subject: [PATCH 01/22] Add Initial Modeling with Through Model and Data Migration for RemoteRepository Model --- .../0011_add_remote_relation_model.py | 48 ++++++++++++++++++ ...012_add_fields_to_remote_relation_model.py | 49 +++++++++++++++++++ ...ata_migration_for_remote_relation_model.py | 49 +++++++++++++++++++ ...move_field_from_remote_repository_model.py | 29 +++++++++++ readthedocs/oauth/models.py | 47 +++++++++++++----- 5 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 readthedocs/oauth/migrations/0011_add_remote_relation_model.py create mode 100644 readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py create mode 100644 readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py create mode 100644 readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py new file mode 100644 index 00000000000..f2241be8062 --- /dev/null +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -0,0 +1,48 @@ +# Generated by Django 2.2.16 on 2020-10-05 06:10 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('oauth', '0010_index_full_name'), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.CreateModel( + name='RemoteRelation', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ], + options={ + 'db_table': 'oauth_remoterepository_users', + }, + ), + migrations.AlterField( + model_name='remoterepository', + name='users', + field=models.ManyToManyField(related_name='oauth_repositories', through='oauth.RemoteRelation', to=settings.AUTH_USER_MODEL, verbose_name='Users'), + ), + migrations.AddField( + model_name='remoterelation', + name='remoterepository', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='remote_relations', to='oauth.RemoteRepository'), + ), + migrations.AddField( + model_name='remoterelation', + name='user', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='remote_relations', to=settings.AUTH_USER_MODEL), + ), + migrations.AlterUniqueTogether( + name='remoterelation', + unique_together={('remoterepository', 'user')}, + ), + ] + ) + ] diff --git a/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py b/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py new file mode 100644 index 00000000000..144830c3cf4 --- /dev/null +++ b/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py @@ -0,0 +1,49 @@ +# Generated by Django 2.2.16 on 2020-10-05 06:13 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('socialaccount', '0003_extra_data_default_dict'), + ('oauth', '0011_add_remote_relation_model'), + ] + + operations = [ + migrations.AddField( + model_name='remoterelation', + name='account', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='remote_relations', to='socialaccount.SocialAccount', verbose_name='Connected account'), + ), + migrations.AddField( + model_name='remoterelation', + name='active', + field=models.BooleanField(default=False, verbose_name='Active'), + ), + migrations.AddField( + model_name='remoterelation', + name='admin', + field=models.BooleanField(default=False, verbose_name='Has admin privilege'), + ), + migrations.AddField( + model_name='remoterelation', + name='json', + field=jsonfield.fields.JSONField(default=dict, verbose_name='Serialized API response'), + preserve_default=False, + ), + migrations.AddField( + model_name='remoterelation', + name='modified_date', + field=models.DateTimeField(auto_now=True, verbose_name='Modified date'), + ), + migrations.AddField( + model_name='remoterelation', + name='pub_date', + field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now, verbose_name='Publication date'), + preserve_default=False, + ), + ] diff --git a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py new file mode 100644 index 00000000000..193df09338f --- /dev/null +++ b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py @@ -0,0 +1,49 @@ +# Generated by Django 2.2.16 on 2020-10-05 06:15 + +import json +from itertools import islice + +from django.db import migrations + + +def move_data_to_remote_relations(apps, schema_editor): + RemoteRelation = apps.get_model('oauth', 'RemoteRelation') + + def remote_relations_generator(relations): + for relation in relations: + relation.account = relation.remoterepository.account + relation.active = relation.remoterepository.active + relation.admin = relation.remoterepository.admin + relation.pub_date = relation.remoterepository.pub_date + try: + relation.json = json.loads(relation.remoterepository.json) + except json.decoder.JSONDecodeError: + pass + + yield relation + + relations_queryset = RemoteRelation.objects.all().select_related('remoterepository') + remote_relations = remote_relations_generator(relations_queryset) + batch_size = 5000 + + while True: + batch = list(islice(remote_relations, batch_size)) + + if not batch: + break + RemoteRelation.objects.bulk_update( + batch, + ['account', 'active', 'admin', 'pub_date', 'json'], + batch_size + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('oauth', '0012_add_fields_to_remote_relation_model'), + ] + + operations = [ + migrations.RunPython(move_data_to_remote_relations), + ] diff --git a/readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py b/readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py new file mode 100644 index 00000000000..0e9082cbe97 --- /dev/null +++ b/readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py @@ -0,0 +1,29 @@ +# Generated by Django 2.2.16 on 2020-10-05 06:18 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('oauth', '0013_data_migration_for_remote_relation_model'), + ] + + operations = [ + migrations.RemoveField( + model_name='remoterepository', + name='account', + ), + migrations.RemoveField( + model_name='remoterepository', + name='active', + ), + migrations.RemoveField( + model_name='remoterepository', + name='admin', + ), + migrations.RemoveField( + model_name='remoterepository', + name='json', + ), + ] diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 027e7e5d634..72387b3fd79 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -11,6 +11,7 @@ from django.db.models import Q from django.urls import reverse from django.utils.translation import ugettext_lazy as _ +from jsonfield import JSONField from readthedocs.projects.constants import REPO_CHOICES from readthedocs.projects.models import Project @@ -90,14 +91,7 @@ class RemoteRepository(models.Model): User, verbose_name=_('Users'), related_name='oauth_repositories', - ) - account = models.ForeignKey( - SocialAccount, - verbose_name=_('Connected account'), - related_name='remote_repositories', - null=True, - blank=True, - on_delete=models.CASCADE, + through='RemoteRelation' ) organization = models.ForeignKey( RemoteOrganization, @@ -107,8 +101,6 @@ class RemoteRepository(models.Model): blank=True, on_delete=models.CASCADE, ) - active = models.BooleanField(_('Active'), default=False) - project = models.OneToOneField( Project, on_delete=models.SET_NULL, @@ -151,7 +143,6 @@ class RemoteRepository(models.Model): html_url = models.URLField(_('HTML URL'), null=True, blank=True) private = models.BooleanField(_('Private repository'), default=False) - admin = models.BooleanField(_('Has admin privilege'), default=False) vcs = models.CharField( _('vcs'), max_length=200, @@ -159,8 +150,6 @@ class RemoteRepository(models.Model): choices=REPO_CHOICES, ) - json = models.TextField(_('Serialized API response')) - objects = RemoteRepositoryQuerySet.as_manager() class Meta: @@ -206,3 +195,35 @@ def matches(self, user): }, ), } for project in projects] + + +class RemoteRelation(models.Model): + remoterepository = models.ForeignKey( + RemoteRepository, + related_name='remote_relations', + on_delete=models.CASCADE + ) + user = models.ForeignKey( + User, + related_name='remote_relations', + on_delete=models.CASCADE + ) + account = models.ForeignKey( + SocialAccount, + verbose_name=_('Connected account'), + related_name='remote_relations', + null=True, + blank=True, + on_delete=models.SET_NULL, + ) + active = models.BooleanField(_('Active'), default=False) + admin = models.BooleanField(_('Has admin privilege'), default=False) + json = JSONField(_('Serialized API response')) + + pub_date = models.DateTimeField(_('Publication date'), auto_now_add=True) + modified_date = models.DateTimeField(_('Modified date'), auto_now=True) + + class Meta: + # Use the existing auto generated table for ManyToMany relations + db_table = 'oauth_remoterepository_users' + unique_together = (('remoterepository', 'user'),) From 7bd1a5fea6638d24713d732e53732436f4e6cec8 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 7 Oct 2020 10:50:26 +0600 Subject: [PATCH 02/22] Improve data migration performance --- .../0013_data_migration_for_remote_relation_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py index 193df09338f..d16bca87924 100644 --- a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py @@ -9,9 +9,9 @@ def move_data_to_remote_relations(apps, schema_editor): RemoteRelation = apps.get_model('oauth', 'RemoteRelation') - def remote_relations_generator(relations): - for relation in relations: - relation.account = relation.remoterepository.account + def remote_relations_generator(relations, batch_size): + for relation in relations.iterator(chunk_size=batch_size): + relation.account_id = relation.remoterepository.account_id relation.active = relation.remoterepository.active relation.admin = relation.remoterepository.admin relation.pub_date = relation.remoterepository.pub_date @@ -22,9 +22,9 @@ def remote_relations_generator(relations): yield relation - relations_queryset = RemoteRelation.objects.all().select_related('remoterepository') - remote_relations = remote_relations_generator(relations_queryset) batch_size = 5000 + relations_queryset = RemoteRelation.objects.all().select_related('remoterepository') + remote_relations = remote_relations_generator(relations_queryset, batch_size) while True: batch = list(islice(remote_relations, batch_size)) @@ -33,7 +33,7 @@ def remote_relations_generator(relations): break RemoteRelation.objects.bulk_update( batch, - ['account', 'active', 'admin', 'pub_date', 'json'], + ['account_id', 'active', 'admin', 'pub_date', 'json'], batch_size ) From 9476a36db1d182b6860b646a66a5ddaaf310ef75 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Sat, 10 Oct 2020 20:27:50 +0600 Subject: [PATCH 03/22] Logging, performance optimization for data migration --- ...ata_migration_for_remote_relation_model.py | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py index d16bca87924..05928a1b36b 100644 --- a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py @@ -1,11 +1,15 @@ # Generated by Django 2.2.16 on 2020-10-05 06:15 -import json from itertools import islice +import json +import logging from django.db import migrations +log = logging.getLogger(__name__) + + def move_data_to_remote_relations(apps, schema_editor): RemoteRelation = apps.get_model('oauth', 'RemoteRelation') @@ -18,19 +22,35 @@ def remote_relations_generator(relations, batch_size): try: relation.json = json.loads(relation.remoterepository.json) except json.decoder.JSONDecodeError: - pass + log.warning( + 'Could not migrate json data for remote_repository=%s', + relation.remoterepository_id + ) yield relation + relations_queryset = RemoteRelation.objects.all().select_related( + 'remoterepository' + ).only( + 'account_id', 'active', 'admin', + 'pub_date', 'json', 'remoterepository__account_id', + 'remoterepository__active', 'remoterepository__admin', + 'remoterepository__pub_date', 'remoterepository__json' + ) + batch_size = 5000 - relations_queryset = RemoteRelation.objects.all().select_related('remoterepository') - remote_relations = remote_relations_generator(relations_queryset, batch_size) + remote_relations = remote_relations_generator( + relations_queryset, batch_size + ) + # Follows Example from django docs + # https://docs.djangoproject.com/en/2.2/ref/models/querysets/#bulk-create while True: batch = list(islice(remote_relations, batch_size)) if not batch: break + RemoteRelation.objects.bulk_update( batch, ['account_id', 'active', 'admin', 'pub_date', 'json'], From 1bd03c77d4d744b03484979a3b7e751b110622d6 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Sat, 10 Oct 2020 21:45:31 +0600 Subject: [PATCH 04/22] Use TimeStampedModel model and follow django docs for migrating through model --- .../0011_add_remote_relation_model.py | 116 +++++++++++++++--- ...012_add_fields_to_remote_relation_model.py | 49 -------- ...ta_migration_for_remote_relation_model.py} | 21 ++-- ...ove_field_from_remote_repository_model.py} | 4 +- readthedocs/oauth/models.py | 11 +- 5 files changed, 118 insertions(+), 83 deletions(-) delete mode 100644 readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py rename readthedocs/oauth/migrations/{0013_data_migration_for_remote_relation_model.py => 0012_data_migration_for_remote_relation_model.py} (73%) rename readthedocs/oauth/migrations/{0014_remove_field_from_remote_repository_model.py => 0013_remove_field_from_remote_repository_model.py} (83%) diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py index f2241be8062..562bbb4fc11 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -1,9 +1,12 @@ -# Generated by Django 2.2.16 on 2020-10-05 06:10 +# Generated by Django 2.2.16 on 2020-10-10 14:55 from django.conf import settings from django.db import migrations, models import django.db.models.deletion +import django_extensions.db.fields +import jsonfield.fields + class Migration(migrations.Migration): @@ -14,35 +17,112 @@ class Migration(migrations.Migration): operations = [ migrations.SeparateDatabaseAndState( + database_operations=[ + migrations.RunSQL( + sql='ALTER TABLE oauth_remoterepository_users RENAME TO oauth_remoterelation', + reverse_sql='ALTER TABLE oauth_remoterelation RENAME TO oauth_remoterepository_users', + ), + ], state_operations=[ migrations.CreateModel( name='RemoteRelation', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ( + 'id', + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ( + 'user', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='remote_relations', + to=settings.AUTH_USER_MODEL + ), + ), + ( + 'remoterepository', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='remote_relations', + to='oauth.RemoteRepository' + ), + ), ], - options={ - 'db_table': 'oauth_remoterepository_users', - }, ), migrations.AlterField( model_name='remoterepository', name='users', - field=models.ManyToManyField(related_name='oauth_repositories', through='oauth.RemoteRelation', to=settings.AUTH_USER_MODEL, verbose_name='Users'), - ), - migrations.AddField( - model_name='remoterelation', - name='remoterepository', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='remote_relations', to='oauth.RemoteRepository'), - ), - migrations.AddField( - model_name='remoterelation', - name='user', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='remote_relations', to=settings.AUTH_USER_MODEL), + field=models.ManyToManyField( + related_name='oauth_repositories', + through='oauth.RemoteRelation', + to=settings.AUTH_USER_MODEL, + verbose_name='Users' + ), ), migrations.AlterUniqueTogether( name='remoterelation', unique_together={('remoterepository', 'user')}, ), - ] - ) + ], + ), + migrations.AddField( + model_name='remoterelation', + name='account', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='remote_relations', + to='socialaccount.SocialAccount', + verbose_name='Connected account' + ), + ), + migrations.AddField( + model_name='remoterelation', + name='active', + field=models.BooleanField( + default=False, + verbose_name='Active' + ), + ), + migrations.AddField( + model_name='remoterelation', + name='admin', + field=models.BooleanField( + default=False, + verbose_name='Has admin privilege' + ), + ), + migrations.AddField( + model_name='remoterelation', + name='json', + field=jsonfield.fields.JSONField( + default=dict, + verbose_name='Serialized API response' + ), + preserve_default=False, + ), + migrations.AddField( + model_name='remoterelation', + name='created', + field=django_extensions.db.fields.CreationDateTimeField( + auto_now_add=True, + default=django.utils.timezone.now, + verbose_name='created', + ), + preserve_default=False, + ), + migrations.AddField( + model_name='remoterelation', + name='modified', + field=django_extensions.db.fields.ModificationDateTimeField( + auto_now=True, + verbose_name='modified' + ), + ), ] diff --git a/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py b/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py deleted file mode 100644 index 144830c3cf4..00000000000 --- a/readthedocs/oauth/migrations/0012_add_fields_to_remote_relation_model.py +++ /dev/null @@ -1,49 +0,0 @@ -# Generated by Django 2.2.16 on 2020-10-05 06:13 - -from django.db import migrations, models -import django.db.models.deletion -import django.utils.timezone -import jsonfield.fields - - -class Migration(migrations.Migration): - - dependencies = [ - ('socialaccount', '0003_extra_data_default_dict'), - ('oauth', '0011_add_remote_relation_model'), - ] - - operations = [ - migrations.AddField( - model_name='remoterelation', - name='account', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='remote_relations', to='socialaccount.SocialAccount', verbose_name='Connected account'), - ), - migrations.AddField( - model_name='remoterelation', - name='active', - field=models.BooleanField(default=False, verbose_name='Active'), - ), - migrations.AddField( - model_name='remoterelation', - name='admin', - field=models.BooleanField(default=False, verbose_name='Has admin privilege'), - ), - migrations.AddField( - model_name='remoterelation', - name='json', - field=jsonfield.fields.JSONField(default=dict, verbose_name='Serialized API response'), - preserve_default=False, - ), - migrations.AddField( - model_name='remoterelation', - name='modified_date', - field=models.DateTimeField(auto_now=True, verbose_name='Modified date'), - ), - migrations.AddField( - model_name='remoterelation', - name='pub_date', - field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now, verbose_name='Publication date'), - preserve_default=False, - ), - ] diff --git a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py similarity index 73% rename from readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py rename to readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py index 05928a1b36b..c9f3f547171 100644 --- a/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.16 on 2020-10-05 06:15 +# Generated by Django 2.2.16 on 2020-10-10 15:00 from itertools import islice import json @@ -18,7 +18,9 @@ def remote_relations_generator(relations, batch_size): relation.account_id = relation.remoterepository.account_id relation.active = relation.remoterepository.active relation.admin = relation.remoterepository.admin - relation.pub_date = relation.remoterepository.pub_date + relation.created = relation.remoterepository.pub_date + relation.modified = relation.remoterepository.modified_date + try: relation.json = json.loads(relation.remoterepository.json) except json.decoder.JSONDecodeError: @@ -32,10 +34,11 @@ def remote_relations_generator(relations, batch_size): relations_queryset = RemoteRelation.objects.all().select_related( 'remoterepository' ).only( - 'account_id', 'active', 'admin', - 'pub_date', 'json', 'remoterepository__account_id', + 'account_id', 'active', 'admin', 'created', + 'modified', 'json', 'remoterepository__account_id', 'remoterepository__active', 'remoterepository__admin', - 'remoterepository__pub_date', 'remoterepository__json' + 'remoterepository__pub_date', 'remoterepository__json', + 'remoterepository__modified_date' ) batch_size = 5000 @@ -53,7 +56,11 @@ def remote_relations_generator(relations, batch_size): RemoteRelation.objects.bulk_update( batch, - ['account_id', 'active', 'admin', 'pub_date', 'json'], + [ + 'account_id', 'active', + 'admin', 'json', + 'created', 'modified', + ], batch_size ) @@ -61,7 +68,7 @@ def remote_relations_generator(relations, batch_size): class Migration(migrations.Migration): dependencies = [ - ('oauth', '0012_add_fields_to_remote_relation_model'), + ('oauth', '0011_add_remote_relation_model'), ] operations = [ diff --git a/readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py b/readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py similarity index 83% rename from readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py rename to readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py index 0e9082cbe97..59c61a717b8 100644 --- a/readthedocs/oauth/migrations/0014_remove_field_from_remote_repository_model.py +++ b/readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.16 on 2020-10-05 06:18 +# Generated by Django 2.2.16 on 2020-10-10 15:21 from django.db import migrations @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ('oauth', '0013_data_migration_for_remote_relation_model'), + ('oauth', '0012_data_migration_for_remote_relation_model'), ] operations = [ diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 72387b3fd79..9c9e72b5827 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -4,13 +4,15 @@ import json -from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.core.validators import URLValidator from django.db import models from django.db.models import Q from django.urls import reverse from django.utils.translation import ugettext_lazy as _ + +from allauth.socialaccount.models import SocialAccount +from django_extensions.db.models import TimeStampedModel from jsonfield import JSONField from readthedocs.projects.constants import REPO_CHOICES @@ -197,7 +199,7 @@ def matches(self, user): } for project in projects] -class RemoteRelation(models.Model): +class RemoteRelation(TimeStampedModel): remoterepository = models.ForeignKey( RemoteRepository, related_name='remote_relations', @@ -220,10 +222,5 @@ class RemoteRelation(models.Model): admin = models.BooleanField(_('Has admin privilege'), default=False) json = JSONField(_('Serialized API response')) - pub_date = models.DateTimeField(_('Publication date'), auto_now_add=True) - modified_date = models.DateTimeField(_('Modified date'), auto_now=True) - class Meta: - # Use the existing auto generated table for ManyToMany relations - db_table = 'oauth_remoterepository_users' unique_together = (('remoterepository', 'user'),) From c3592eb32c0fe3d2a0256bd5a117b377568ba254 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Sat, 14 Nov 2020 14:54:36 +0600 Subject: [PATCH 05/22] Updated data migrations to only migrate RemoteRepositories of recently loggedin users --- ...ata_migration_for_remote_relation_model.py | 8 +++-- ...move_field_from_remote_repository_model.py | 29 ------------------- readthedocs/oauth/models.py | 5 ++++ 3 files changed, 10 insertions(+), 32 deletions(-) delete mode 100644 readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py diff --git a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py index c9f3f547171..840b02150cc 100644 --- a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py @@ -5,7 +5,7 @@ import logging from django.db import migrations - +from django.utils import timezone log = logging.getLogger(__name__) @@ -31,7 +31,9 @@ def remote_relations_generator(relations, batch_size): yield relation - relations_queryset = RemoteRelation.objects.all().select_related( + relations_queryset = RemoteRelation.objects.filter( + user__last_login__gte=timezone.now() - timezone.timedelta(days=30) + ).select_related( 'remoterepository' ).only( 'account_id', 'active', 'admin', 'created', @@ -41,7 +43,7 @@ def remote_relations_generator(relations, batch_size): 'remoterepository__modified_date' ) - batch_size = 5000 + batch_size = 1000 remote_relations = remote_relations_generator( relations_queryset, batch_size ) diff --git a/readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py b/readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py deleted file mode 100644 index 59c61a717b8..00000000000 --- a/readthedocs/oauth/migrations/0013_remove_field_from_remote_repository_model.py +++ /dev/null @@ -1,29 +0,0 @@ -# Generated by Django 2.2.16 on 2020-10-10 15:21 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('oauth', '0012_data_migration_for_remote_relation_model'), - ] - - operations = [ - migrations.RemoveField( - model_name='remoterepository', - name='account', - ), - migrations.RemoveField( - model_name='remoterepository', - name='active', - ), - migrations.RemoveField( - model_name='remoterepository', - name='admin', - ), - migrations.RemoveField( - model_name='remoterepository', - name='json', - ), - ] diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 9c9e72b5827..6a8790fe8b6 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -103,6 +103,8 @@ class RemoteRepository(models.Model): blank=True, on_delete=models.CASCADE, ) + active = models.BooleanField(_('Active'), default=False) + project = models.OneToOneField( Project, on_delete=models.SET_NULL, @@ -145,6 +147,7 @@ class RemoteRepository(models.Model): html_url = models.URLField(_('HTML URL'), null=True, blank=True) private = models.BooleanField(_('Private repository'), default=False) + admin = models.BooleanField(_('Has admin privilege'), default=False) vcs = models.CharField( _('vcs'), max_length=200, @@ -152,6 +155,8 @@ class RemoteRepository(models.Model): choices=REPO_CHOICES, ) + json = models.TextField(_('Serialized API response')) + objects = RemoteRepositoryQuerySet.as_manager() class Meta: From c6699fb25ba9c3cb391adce77e66f2ce237b7a5f Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Sat, 14 Nov 2020 15:00:24 +0600 Subject: [PATCH 06/22] Do not remove fields from RemoteRepository Model --- readthedocs/oauth/models.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 6a8790fe8b6..74dae34c0a6 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -95,6 +95,14 @@ class RemoteRepository(models.Model): related_name='oauth_repositories', through='RemoteRelation' ) + account = models.ForeignKey( + SocialAccount, + verbose_name=_('Connected account'), + related_name='remote_repositories', + null=True, + blank=True, + on_delete=models.CASCADE, + ) organization = models.ForeignKey( RemoteOrganization, verbose_name=_('Organization'), From bf5c6b1213816a572b8e626c7ee95c839fa5c0e8 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Mon, 16 Nov 2020 21:30:09 +0600 Subject: [PATCH 07/22] Do not Migrate Active Field --- .../oauth/migrations/0011_add_remote_relation_model.py | 8 -------- .../0012_data_migration_for_remote_relation_model.py | 10 ++++------ readthedocs/oauth/models.py | 1 - 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py index 562bbb4fc11..4660e4d4333 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -82,14 +82,6 @@ class Migration(migrations.Migration): verbose_name='Connected account' ), ), - migrations.AddField( - model_name='remoterelation', - name='active', - field=models.BooleanField( - default=False, - verbose_name='Active' - ), - ), migrations.AddField( model_name='remoterelation', name='admin', diff --git a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py index 840b02150cc..028aa36654c 100644 --- a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py @@ -16,7 +16,6 @@ def move_data_to_remote_relations(apps, schema_editor): def remote_relations_generator(relations, batch_size): for relation in relations.iterator(chunk_size=batch_size): relation.account_id = relation.remoterepository.account_id - relation.active = relation.remoterepository.active relation.admin = relation.remoterepository.admin relation.created = relation.remoterepository.pub_date relation.modified = relation.remoterepository.modified_date @@ -36,11 +35,10 @@ def remote_relations_generator(relations, batch_size): ).select_related( 'remoterepository' ).only( - 'account_id', 'active', 'admin', 'created', + 'account_id', 'admin', 'created', 'modified', 'json', 'remoterepository__account_id', - 'remoterepository__active', 'remoterepository__admin', - 'remoterepository__pub_date', 'remoterepository__json', - 'remoterepository__modified_date' + 'remoterepository__admin', 'remoterepository__pub_date', + 'remoterepository__json', 'remoterepository__modified_date' ) batch_size = 1000 @@ -59,7 +57,7 @@ def remote_relations_generator(relations, batch_size): RemoteRelation.objects.bulk_update( batch, [ - 'account_id', 'active', + 'account_id', 'admin', 'json', 'created', 'modified', ], diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 74dae34c0a6..d429eebbf0d 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -231,7 +231,6 @@ class RemoteRelation(TimeStampedModel): blank=True, on_delete=models.SET_NULL, ) - active = models.BooleanField(_('Active'), default=False) admin = models.BooleanField(_('Has admin privilege'), default=False) json = JSONField(_('Serialized API response')) From d90ecadbee8bf4cea73e3319166841b06253306b Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Mon, 16 Nov 2020 22:41:19 +0600 Subject: [PATCH 08/22] Rename RemoteRelation model to RemoteRepositoryRelation --- .../0011_add_remote_relation_model.py | 20 +++++++++---------- ...ata_migration_for_remote_relation_model.py | 6 +++--- readthedocs/oauth/models.py | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py index 4660e4d4333..10a4ccd620d 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -19,13 +19,13 @@ class Migration(migrations.Migration): migrations.SeparateDatabaseAndState( database_operations=[ migrations.RunSQL( - sql='ALTER TABLE oauth_remoterepository_users RENAME TO oauth_remoterelation', - reverse_sql='ALTER TABLE oauth_remoterelation RENAME TO oauth_remoterepository_users', + sql='ALTER TABLE oauth_remoterepository_users RENAME TO oauth_remoterepositoryrelation', + reverse_sql='ALTER TABLE oauth_remoterepositoryrelation RENAME TO oauth_remoterepository_users', ), ], state_operations=[ migrations.CreateModel( - name='RemoteRelation', + name='RemoteRepositoryRelation', fields=[ ( 'id', @@ -59,19 +59,19 @@ class Migration(migrations.Migration): name='users', field=models.ManyToManyField( related_name='oauth_repositories', - through='oauth.RemoteRelation', + through='oauth.RemoteRepositoryRelation', to=settings.AUTH_USER_MODEL, verbose_name='Users' ), ), migrations.AlterUniqueTogether( - name='remoterelation', + name='remoterepositoryrelation', unique_together={('remoterepository', 'user')}, ), ], ), migrations.AddField( - model_name='remoterelation', + model_name='remoterepositoryrelation', name='account', field=models.ForeignKey( blank=True, @@ -83,7 +83,7 @@ class Migration(migrations.Migration): ), ), migrations.AddField( - model_name='remoterelation', + model_name='remoterepositoryrelation', name='admin', field=models.BooleanField( default=False, @@ -91,7 +91,7 @@ class Migration(migrations.Migration): ), ), migrations.AddField( - model_name='remoterelation', + model_name='remoterepositoryrelation', name='json', field=jsonfield.fields.JSONField( default=dict, @@ -100,7 +100,7 @@ class Migration(migrations.Migration): preserve_default=False, ), migrations.AddField( - model_name='remoterelation', + model_name='remoterepositoryrelation', name='created', field=django_extensions.db.fields.CreationDateTimeField( auto_now_add=True, @@ -110,7 +110,7 @@ class Migration(migrations.Migration): preserve_default=False, ), migrations.AddField( - model_name='remoterelation', + model_name='remoterepositoryrelation', name='modified', field=django_extensions.db.fields.ModificationDateTimeField( auto_now=True, diff --git a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py index 028aa36654c..ba69fca4f58 100644 --- a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py @@ -11,7 +11,7 @@ def move_data_to_remote_relations(apps, schema_editor): - RemoteRelation = apps.get_model('oauth', 'RemoteRelation') + RemoteRepositoryRelation = apps.get_model('oauth', 'RemoteRepositoryRelation') def remote_relations_generator(relations, batch_size): for relation in relations.iterator(chunk_size=batch_size): @@ -30,7 +30,7 @@ def remote_relations_generator(relations, batch_size): yield relation - relations_queryset = RemoteRelation.objects.filter( + relations_queryset = RemoteRepositoryRelation.objects.filter( user__last_login__gte=timezone.now() - timezone.timedelta(days=30) ).select_related( 'remoterepository' @@ -54,7 +54,7 @@ def remote_relations_generator(relations, batch_size): if not batch: break - RemoteRelation.objects.bulk_update( + RemoteRepositoryRelation.objects.bulk_update( batch, [ 'account_id', diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index d429eebbf0d..6f459a87306 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -93,7 +93,7 @@ class RemoteRepository(models.Model): User, verbose_name=_('Users'), related_name='oauth_repositories', - through='RemoteRelation' + through='RemoteRepositoryRelation' ) account = models.ForeignKey( SocialAccount, @@ -212,7 +212,7 @@ def matches(self, user): } for project in projects] -class RemoteRelation(TimeStampedModel): +class RemoteRepositoryRelation(TimeStampedModel): remoterepository = models.ForeignKey( RemoteRepository, related_name='remote_relations', From 9f300591702ba1b1c199f8342d5722f8852c5e40 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Wed, 18 Nov 2020 21:50:20 +0600 Subject: [PATCH 09/22] Update oauth services to use the RemoteRepositoryRelation model --- readthedocs/api/v2/views/model_views.py | 4 +- readthedocs/oauth/admin.py | 7 ++- readthedocs/oauth/models.py | 18 ++++---- readthedocs/oauth/services/bitbucket.py | 45 ++++++++++++------- readthedocs/oauth/services/github.py | 49 +++++++++++--------- readthedocs/oauth/services/gitlab.py | 60 ++++++++++++++----------- 6 files changed, 107 insertions(+), 76 deletions(-) diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index c04e0b5d9e2..a089e60c1c7 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -397,10 +397,10 @@ def get_queryset(self): ) query = query.filter( - account__provider__in=[ + remote_relations__account__provider__in=[ service.adapter.provider_id for service in registry ], - ) + ).distinct() # optimizes for the RemoteOrganizationSerializer query = query.select_related('organization') diff --git a/readthedocs/oauth/admin.py b/readthedocs/oauth/admin.py index eeee459e6ec..9de27927a1c 100644 --- a/readthedocs/oauth/admin.py +++ b/readthedocs/oauth/admin.py @@ -4,7 +4,11 @@ from django.contrib import admin -from .models import RemoteOrganization, RemoteRepository +from .models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) class RemoteRepositoryAdmin(admin.ModelAdmin): @@ -22,4 +26,5 @@ class RemoteOrganizationAdmin(admin.ModelAdmin): admin.site.register(RemoteRepository, RemoteRepositoryAdmin) +admin.site.register(RemoteRepositoryRelation) admin.site.register(RemoteOrganization, RemoteOrganizationAdmin) diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 6f459a87306..50b3240c734 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -174,15 +174,6 @@ class Meta: def __str__(self): return 'Remote repository: {}'.format(self.html_url) - def get_serialized(self, key=None, default=None): - try: - data = json.loads(self.json) - if key is not None: - return data.get(key, default) - return data - except ValueError: - pass - @property def clone_fuzzy_url(self): """Try to match against several permutations of project URL.""" @@ -236,3 +227,12 @@ class RemoteRepositoryRelation(TimeStampedModel): class Meta: unique_together = (('remoterepository', 'user'),) + + def get_serialized(self, key=None, default=None): + try: + data = self.json + if key is not None: + return data.get(key, default) + return data + except ValueError: + pass diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index accd2255a69..2d76d7c1888 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -14,7 +14,11 @@ from readthedocs.builds import utils as build_utils from readthedocs.integrations.models import Integration -from ..models import RemoteOrganization, RemoteRepository +from ..models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) from .base import Service, SyncServiceError @@ -57,16 +61,18 @@ def sync_repositories(self): resp = self.paginate( 'https://bitbucket.org/api/2.0/repositories/?role=admin', ) - admin_repos = ( - RemoteRepository.objects.filter( - users=self.user, - full_name__in=[r['full_name'] for r in resp], + admin_repo_relations = ( + RemoteRepositoryRelation.objects.filter( + user=self.user, account=self.account, + remoterepository__full_name__in=[ + r['full_name'] for r in resp + ] ) ) - for repo in admin_repos: - repo.admin = True - repo.save() + for remote_relation in admin_repo_relations: + remote_relation.admin = True + remote_relation.save() except (TypeError, ValueError): pass @@ -120,13 +126,18 @@ def create_repository(self, fields, privacy=None, organization=None): """ privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL if any([ - (privacy == 'private'), - (fields['is_private'] is False and privacy == 'public'), + (privacy == 'private'), + (fields['is_private'] is False and privacy == 'public'), ]): - repo, _ = RemoteRepository.objects.get_or_create( - full_name=fields['full_name'], - account=self.account, + repo, _ = RemoteRepository.objects.get_or_create( + full_name=fields['full_name'] + ) + remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( + remoterepository=repo, + user=self.user, + account=self.account ) + if repo.organization and repo.organization != organization: log.debug( 'Not importing %s because mismatched orgs', @@ -135,7 +146,6 @@ def create_repository(self, fields, privacy=None, organization=None): return None repo.organization = organization - repo.users.add(self.user) repo.name = fields['name'] repo.description = fields['description'] repo.private = fields['is_private'] @@ -155,15 +165,18 @@ def create_repository(self, fields, privacy=None, organization=None): repo.html_url = fields['links']['html']['href'] repo.vcs = fields['scm'] - repo.account = self.account avatar_url = fields['links']['avatar']['href'] or '' repo.avatar_url = re.sub(r'\/16\/$', r'/32/', avatar_url) if not repo.avatar_url: repo.avatar_url = self.default_user_avatar_url - repo.json = json.dumps(fields) repo.save() + + remote_relation.account = self.account + remote_relation.json = fields + remote_relation.save() + return repo log.debug( diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 14a480b2330..734ce1df265 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -8,7 +8,6 @@ from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter from django.conf import settings -from django.db.models import Q from django.urls import reverse from requests.exceptions import RequestException @@ -20,7 +19,11 @@ ) from readthedocs.integrations.models import Integration -from ..models import RemoteOrganization, RemoteRepository +from ..models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) from .base import Service, SyncServiceError log = logging.getLogger(__name__) @@ -97,21 +100,19 @@ def create_repository(self, fields, privacy=None, organization=None): """ privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL if any([ - (privacy == 'private'), - (fields['private'] is False and privacy == 'public'), + (privacy == 'private'), + (fields['private'] is False and privacy == 'public'), ]): - try: - repo = RemoteRepository.objects.get( - full_name=fields['full_name'], - users=self.user, - account=self.account, - ) - except RemoteRepository.DoesNotExist: - repo = RemoteRepository.objects.create( - full_name=fields['full_name'], - account=self.account, - ) - repo.users.add(self.user) + + repo, _ = RemoteRepository.objects.get_or_create( + full_name=fields['full_name'] + ) + remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( + remoterepository=repo, + user=self.user, + account=self.account + ) + if repo.organization and repo.organization != organization: log.debug( 'Not importing %s because mismatched orgs', @@ -125,20 +126,26 @@ def create_repository(self, fields, privacy=None, organization=None): repo.ssh_url = fields['ssh_url'] repo.html_url = fields['html_url'] repo.private = fields['private'] + repo.vcs = 'git' + repo.avatar_url = fields.get('owner', {}).get('avatar_url') + if repo.private: repo.clone_url = fields['ssh_url'] else: repo.clone_url = fields['clone_url'] - repo.admin = fields.get('permissions', {}).get('admin', False) - repo.vcs = 'git' - repo.account = self.account - repo.avatar_url = fields.get('owner', {}).get('avatar_url') + if not repo.avatar_url: repo.avatar_url = self.default_user_avatar_url - repo.json = json.dumps(fields) + repo.save() + + remote_relation.json = fields + remote_relation.admin = fields.get('permissions', {}).get('admin', False) + remote_relation.save() + return repo + log.debug( 'Not importing %s because mismatched type', fields['name'], diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 2d853fac566..57af3276a5b 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -3,7 +3,7 @@ import json import logging import re -from urllib.parse import urljoin, urlparse +from urllib.parse import urlparse from allauth.socialaccount.providers.gitlab.views import GitLabOAuth2Adapter from django.conf import settings @@ -18,7 +18,11 @@ from readthedocs.integrations.models import Integration from readthedocs.projects.models import Project -from ..models import RemoteOrganization, RemoteRepository +from ..models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) from .base import Service, SyncServiceError log = logging.getLogger(__name__) @@ -159,18 +163,14 @@ def create_repository(self, fields, privacy=None, organization=None): privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL repo_is_public = fields['visibility'] == 'public' if privacy == 'private' or (repo_is_public and privacy == 'public'): - try: - repo = RemoteRepository.objects.get( - full_name=fields['name_with_namespace'], - users=self.user, - account=self.account, - ) - except RemoteRepository.DoesNotExist: - repo = RemoteRepository.objects.create( - full_name=fields['name_with_namespace'], - account=self.account, - ) - repo.users.add(self.user) + repo, _ = RemoteRepository.objects.get_or_create( + full_name=fields['name_with_namespace'] + ) + remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( + remoterepository=repo, + user=self.user, + account=self.account + ) if repo.organization and repo.organization != organization: log.debug( @@ -184,36 +184,42 @@ def create_repository(self, fields, privacy=None, organization=None): repo.description = fields['description'] repo.ssh_url = fields['ssh_url_to_repo'] repo.html_url = fields['web_url'] + repo.vcs = 'git' repo.private = not repo_is_public + + owner = fields.get('owner') or {} + repo.avatar_url = ( + fields.get('avatar_url') or owner.get('avatar_url') + ) + + if not repo.avatar_url: + repo.avatar_url = self.default_user_avatar_url + if repo.private: repo.clone_url = repo.ssh_url else: repo.clone_url = fields['http_url_to_repo'] + repo.save() + project_access_level = group_access_level = self.PERMISSION_NO_ACCESS + project_access = fields.get('permissions', {}).get('project_access', {}) if project_access: project_access_level = project_access.get('access_level', self.PERMISSION_NO_ACCESS) + group_access = fields.get('permissions', {}).get('group_access', {}) if group_access: group_access_level = group_access.get('access_level', self.PERMISSION_NO_ACCESS) - repo.admin = any([ + + remote_relation.admin = any([ project_access_level in (self.PERMISSION_MAINTAINER, self.PERMISSION_OWNER), group_access_level in (self.PERMISSION_MAINTAINER, self.PERMISSION_OWNER), ]) + remote_relation.account = self.account + remote_relation.json = fields + remote_relation.save() - repo.vcs = 'git' - repo.account = self.account - - owner = fields.get('owner') or {} - repo.avatar_url = ( - fields.get('avatar_url') or owner.get('avatar_url') - ) - if not repo.avatar_url: - repo.avatar_url = self.default_user_avatar_url - - repo.json = json.dumps(fields) - repo.save() return repo log.info( From 42a516de6bf06192cb72d889847fd15abff4eb05 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Thu, 19 Nov 2020 20:18:00 +0600 Subject: [PATCH 10/22] Use remote relations on update_webhook function --- readthedocs/oauth/services/base.py | 2 +- readthedocs/oauth/utils.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 1040fcade27..231150fdd0d 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -206,7 +206,7 @@ def sync(self): .exclude( Q(full_name__in=repository_full_names) | Q(project__isnull=False) ) - .filter(account=self.account) + .filter(remote_relations__account=self.account) .delete() ) diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index babb766488b..fef10b7d8ce 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -31,9 +31,17 @@ def update_webhook(project, integration, request=None): updated = False try: - account = project.remote_repository.account - service = service_cls(request.user, account) - updated, __ = service.update_webhook(project, integration) + remote_relations = project.remote_repository.remote_relations.filter( + account__isnull=False + ) + + for relation in remote_relations: + service = service_cls(request.user, relation.account) + updated, __ = service.update_webhook(project, integration) + + if updated: + break + except Project.remote_repository.RelatedObjectDoesNotExist: # The project was imported manually and doesn't have a RemoteRepository # attached. We do brute force over all the accounts registered for this From 48114e64e16f755228cad7245a936d0d092af02e Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Thu, 19 Nov 2020 20:27:02 +0600 Subject: [PATCH 11/22] Lint fix --- readthedocs/oauth/services/bitbucket.py | 2 +- readthedocs/oauth/services/github.py | 3 +-- readthedocs/oauth/services/gitlab.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 2d76d7c1888..7b1f5b0b0f0 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -129,7 +129,7 @@ def create_repository(self, fields, privacy=None, organization=None): (privacy == 'private'), (fields['is_private'] is False and privacy == 'public'), ]): - repo, _ = RemoteRepository.objects.get_or_create( + repo, _ = RemoteRepository.objects.get_or_create( full_name=fields['full_name'] ) remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 734ce1df265..30210010521 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -104,7 +104,7 @@ def create_repository(self, fields, privacy=None, organization=None): (fields['private'] is False and privacy == 'public'), ]): - repo, _ = RemoteRepository.objects.get_or_create( + repo, _ = RemoteRepository.objects.get_or_create( full_name=fields['full_name'] ) remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( @@ -145,7 +145,6 @@ def create_repository(self, fields, privacy=None, organization=None): return repo - log.debug( 'Not importing %s because mismatched type', fields['name'], diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 57af3276a5b..6467a98631f 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -163,7 +163,7 @@ def create_repository(self, fields, privacy=None, organization=None): privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL repo_is_public = fields['visibility'] == 'public' if privacy == 'private' or (repo_is_public and privacy == 'public'): - repo, _ = RemoteRepository.objects.get_or_create( + repo, _ = RemoteRepository.objects.get_or_create( full_name=fields['name_with_namespace'] ) remote_relation, _ = RemoteRepositoryRelation.objects.get_or_create( From 421bcaeb3bbd8b55dc0624f560273ac425a85089 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Thu, 19 Nov 2020 23:57:13 +0600 Subject: [PATCH 12/22] Fix tests --- readthedocs/api/v2/serializers.py | 2 +- readthedocs/rtd_tests/tests/test_api.py | 28 ++++++-- readthedocs/rtd_tests/tests/test_oauth.py | 5 +- .../rtd_tests/tests/test_oauth_sync.py | 64 +++++++++++++++---- 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index f26a97640bc..b38ebc4eed4 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -186,7 +186,7 @@ class RemoteRepositorySerializer(serializers.ModelSerializer): class Meta: model = RemoteRepository - exclude = ('json', 'users') + exclude = ('users',) def get_matches(self, obj): request = self.context['request'] diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index dc4298ae4a5..d3ab852e57a 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -40,7 +40,7 @@ from readthedocs.builds.constants import LATEST, EXTERNAL from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.integrations.models import Integration -from readthedocs.oauth.models import RemoteOrganization, RemoteRepository +from readthedocs.oauth.models import RemoteOrganization, RemoteRepository, RemoteRepositoryRelation from readthedocs.projects.models import ( APIProject, EnvironmentVariable, @@ -650,8 +650,15 @@ def test_project_pagination(self): def test_remote_repository_pagination(self): account = get(SocialAccount, provider='github') user = get(User) + for _ in range(20): - get(RemoteRepository, users=[user], account=account) + repo = get(RemoteRepository) + get( + RemoteRepositoryRelation, + remoterepository=repo, + user=user, + account=account + ) client = APIClient() client.force_authenticate(user=user) @@ -765,15 +772,24 @@ def test_permissions(self): org_a = get(RemoteOrganization, users=[user_a], account=account_a) repo_a = get( RemoteRepository, - users=[user_a], organization=org_a, - account=account_a, ) + get( + RemoteRepositoryRelation, + remoterepository=repo_a, + user=user_a, + account=account_a + ) + repo_b = get( RemoteRepository, - users=[user_b], organization=None, - account=account_b, + ) + get( + RemoteRepositoryRelation, + remoterepository=repo_b, + user=user_b, + account=account_b ) client.force_authenticate(user=user_a) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 13c002cbbbf..1e890a5cfdc 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -10,7 +10,6 @@ from readthedocs.builds.constants import BUILD_STATUS_SUCCESS, EXTERNAL from readthedocs.builds.models import Build, Version from readthedocs.integrations.models import ( - BitbucketWebhook, GitHubWebhook, GitLabWebhook, ) @@ -147,7 +146,7 @@ def test_multiple_users_same_repo(self): ) self.assertIsInstance(github_project, RemoteRepository) self.assertIsInstance(github_project_2, RemoteRepository) - self.assertNotEqual(github_project_2, github_project) + self.assertEqual(github_project_2, github_project) github_project_3 = self.service.create_repository( repo_json, organization=self.org, privacy=self.privacy, @@ -971,7 +970,7 @@ def test_make_project_pass(self): ) self.assertEqual(repo.ssh_url, 'git@gitlab.com:testorga/testrepo.git') self.assertEqual(repo.html_url, 'https://gitlab.com/testorga/testrepo') - self.assertTrue(repo.admin) + self.assertTrue(repo.remote_relations.first().admin) self.assertFalse(repo.private) def test_make_private_project_fail(self): diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py index 0c466845997..c1740587ff3 100644 --- a/readthedocs/rtd_tests/tests/test_oauth_sync.py +++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py @@ -1,15 +1,12 @@ -from unittest import mock - from allauth.socialaccount.models import SocialAccount, SocialToken from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter import django_dynamic_fixture as fixture import requests_mock -from django.conf import settings from django.contrib.auth.models import User from django.test import TestCase -from readthedocs.oauth.models import RemoteOrganization, RemoteRepository +from readthedocs.oauth.models import RemoteOrganization, RemoteRepository, RemoteRepositoryRelation from readthedocs.oauth.services import ( GitHubService, ) @@ -75,28 +72,41 @@ def test_sync_delete_stale(self, mock_request): mock_request.get('https://api.github.com/user/repos', json=self.payload_user_repos) mock_request.get('https://api.github.com/user/orgs', json=[]) - fixture.get( + repo_1 = fixture.get( RemoteRepository, - account=self.socialaccount, - users=[self.user], full_name='organization/repository', ) fixture.get( + RemoteRepositoryRelation, + remoterepository=repo_1, + user=self.user, + account=self.socialaccount + ) + + repo_2 = fixture.get( RemoteRepository, - account=self.socialaccount, - users=[self.user], full_name='organization/old-repository', ) + fixture.get( + RemoteRepositoryRelation, + remoterepository=repo_2, + user=self.user, + account=self.socialaccount + ) # RemoteRepository linked to a Project shouldn't be removed project = fixture.get(Project) - fixture.get( + repo_3 = fixture.get( RemoteRepository, - account=self.socialaccount, project=project, - users=[self.user], full_name='organization/project-linked-repository', ) + fixture.get( + RemoteRepositoryRelation, + remoterepository=repo_3, + user=self.user, + account=self.socialaccount + ) fixture.get( RemoteOrganization, @@ -137,6 +147,36 @@ def test_sync_repositories(self, mock_request): self.assertFalse(remote_repository.admin) self.assertFalse(remote_repository.private) + @requests_mock.Mocker(kw='mock_request') + def test_sync_repositories_only_creates_one_remote_repo_per_vcs_repo(self, mock_request): + mock_request.get('https://api.github.com/user/repos', json=self.payload_user_repos) + + self.assertEqual(RemoteRepository.objects.count(), 0) + + remote_repositories = self.service.sync_repositories() + + self.assertEqual(RemoteRepository.objects.count(), 1) + self.assertEqual(len(remote_repositories), 1) + self.assertEqual(RemoteRepositoryRelation.objects.count(), 1) + + user_2 = fixture.get(User) + user_2_socialaccount = fixture.get( + SocialAccount, + user=user_2, + provider=GitHubOAuth2Adapter.provider_id, + ) + fixture.get( + SocialToken, + account=user_2_socialaccount, + ) + + service_2 = GitHubService(user=user_2, account=user_2_socialaccount) + remote_repositories = service_2.sync_repositories() + + self.assertEqual(RemoteRepository.objects.count(), 1) + self.assertEqual(len(remote_repositories), 1) + self.assertEqual(RemoteRepositoryRelation.objects.count(), 2) + @requests_mock.Mocker(kw='mock_request') def test_sync_organizations(self, mock_request): payload = [{ From afc7af0b047700c72ee7774af6bdac00a0235861 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Fri, 20 Nov 2020 00:00:05 +0600 Subject: [PATCH 13/22] lint fix --- readthedocs/rtd_tests/tests/test_api.py | 7 +++++-- readthedocs/rtd_tests/tests/test_oauth_sync.py | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index d3ab852e57a..df7304c193c 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -26,7 +26,6 @@ GITLAB_MERGE_REQUEST, GITLAB_MERGE_REQUEST_CLOSE, GITLAB_MERGE_REQUEST_MERGE, - GITLAB_MERGE_REQUEST_OPEN, GITLAB_MERGE_REQUEST_REOPEN, GITLAB_MERGE_REQUEST_UPDATE, GITLAB_NULL_HASH, @@ -40,7 +39,11 @@ from readthedocs.builds.constants import LATEST, EXTERNAL from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.integrations.models import Integration -from readthedocs.oauth.models import RemoteOrganization, RemoteRepository, RemoteRepositoryRelation +from readthedocs.oauth.models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) from readthedocs.projects.models import ( APIProject, EnvironmentVariable, diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py index c1740587ff3..5e457bb2544 100644 --- a/readthedocs/rtd_tests/tests/test_oauth_sync.py +++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py @@ -6,7 +6,11 @@ from django.contrib.auth.models import User from django.test import TestCase -from readthedocs.oauth.models import RemoteOrganization, RemoteRepository, RemoteRepositoryRelation +from readthedocs.oauth.models import ( + RemoteOrganization, + RemoteRepository, + RemoteRepositoryRelation, +) from readthedocs.oauth.services import ( GitHubService, ) From 28c6a187c5c84009135651cb4dcc50ee42a04361 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Fri, 20 Nov 2020 20:16:34 +0600 Subject: [PATCH 14/22] Add TODO for removing json field from RemoteRepository serializer --- readthedocs/api/v2/serializers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index b38ebc4eed4..3df9fe6048b 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -186,7 +186,8 @@ class RemoteRepositorySerializer(serializers.ModelSerializer): class Meta: model = RemoteRepository - exclude = ('users',) + #TODO: Remove json field after it is removed from RemoteRepository + exclude = ('json', 'users') def get_matches(self, obj): request = self.context['request'] From 37d89afb9d2f3f8779807df332f7a2de1f97cba1 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Sat, 21 Nov 2020 15:58:30 +0600 Subject: [PATCH 15/22] Optimize remote_relations query --- readthedocs/api/v2/serializers.py | 2 +- readthedocs/oauth/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index 3df9fe6048b..c3a77f535b7 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -186,7 +186,7 @@ class RemoteRepositorySerializer(serializers.ModelSerializer): class Meta: model = RemoteRepository - #TODO: Remove json field after it is removed from RemoteRepository + # TODO: Remove json field after it is removed from RemoteRepository exclude = ('json', 'users') def get_matches(self, obj): diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index fef10b7d8ce..9bdc772c5ea 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -33,7 +33,7 @@ def update_webhook(project, integration, request=None): try: remote_relations = project.remote_repository.remote_relations.filter( account__isnull=False - ) + ).select_related('account') for relation in remote_relations: service = service_cls(request.user, relation.account) From 8d126db3c63005d355c703292d4738b79d54dbdc Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 13:19:54 +0600 Subject: [PATCH 16/22] Filter remote_relations according to loggedin user on update_webhook --- readthedocs/oauth/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index 9bdc772c5ea..0f93d0f0917 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -32,7 +32,8 @@ def update_webhook(project, integration, request=None): updated = False try: remote_relations = project.remote_repository.remote_relations.filter( - account__isnull=False + account__isnull=False, + user=request.user ).select_related('account') for relation in remote_relations: From dff138ef123eb1b9d317381cc7cbbafeab723027 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 13:49:03 +0600 Subject: [PATCH 17/22] Only delete RemoteRepositoryRelation objects on remote repository sync --- readthedocs/oauth/services/base.py | 7 ++++--- readthedocs/rtd_tests/tests/test_oauth_sync.py | 10 ++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 231150fdd0d..76561520fd3 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -202,11 +202,12 @@ def sync(self): all_remote_repositories = remote_repositories + remote_repositories_organizations repository_full_names = [r.full_name for r in all_remote_repositories if r is not None] ( - self.user.oauth_repositories + self.user.remote_relations .exclude( - Q(full_name__in=repository_full_names) | Q(project__isnull=False) + Q(remoterepository__full_name__in=repository_full_names) | + Q(remoterepository__project__isnull=False) ) - .filter(remote_relations__account=self.account) + .filter(account=self.account) .delete() ) diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py index 5e457bb2544..8ae8d02899f 100644 --- a/readthedocs/rtd_tests/tests/test_oauth_sync.py +++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py @@ -120,14 +120,16 @@ def test_sync_delete_stale(self, mock_request): ) self.assertEqual(RemoteRepository.objects.count(), 3) + self.assertEqual(RemoteRepositoryRelation.objects.count(), 3) self.assertEqual(RemoteOrganization.objects.count(), 1) self.service.sync() - # After calling .sync, old-repository should be deleted, - # project-linked-repository should be conserved, and only the one - # returned by the API should be present (organization/repository) - self.assertEqual(RemoteRepository.objects.count(), 2) + # After calling .sync, old-repository remote relation should be deleted, + # project-linked-repository remote relation should be conserved, + # and only the one's returned by the API should be present (organization/repository) + self.assertEqual(RemoteRepository.objects.count(), 3) + self.assertEqual(RemoteRepositoryRelation.objects.count(), 2) self.assertTrue(RemoteRepository.objects.filter(full_name='organization/repository').exists()) self.assertTrue(RemoteRepository.objects.filter(full_name='organization/project-linked-repository').exists()) self.assertEqual(RemoteOrganization.objects.count(), 0) From 5d7c56f544ee4a75b41655621ad417e77b350b65 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 13:55:27 +0600 Subject: [PATCH 18/22] Change RemoteRepositoryRelation related_name to remote_repository_relations --- readthedocs/api/v2/views/model_views.py | 2 +- .../oauth/migrations/0011_add_remote_relation_model.py | 6 +++--- .../0012_data_migration_for_remote_relation_model.py | 10 +++++----- readthedocs/oauth/models.py | 6 +++--- readthedocs/oauth/services/base.py | 2 +- readthedocs/oauth/utils.py | 4 ++-- readthedocs/rtd_tests/tests/test_oauth.py | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index a089e60c1c7..431c0ec3f25 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -397,7 +397,7 @@ def get_queryset(self): ) query = query.filter( - remote_relations__account__provider__in=[ + remote_repository_relations__account__provider__in=[ service.adapter.provider_id for service in registry ], ).distinct() diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py index 10a4ccd620d..4b0f9ea4ed4 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -40,7 +40,7 @@ class Migration(migrations.Migration): 'user', models.ForeignKey( on_delete=django.db.models.deletion.CASCADE, - related_name='remote_relations', + related_name='remote_repository_relations', to=settings.AUTH_USER_MODEL ), ), @@ -48,7 +48,7 @@ class Migration(migrations.Migration): 'remoterepository', models.ForeignKey( on_delete=django.db.models.deletion.CASCADE, - related_name='remote_relations', + related_name='remote_repository_relations', to='oauth.RemoteRepository' ), ), @@ -77,7 +77,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, - related_name='remote_relations', + related_name='remote_repository_relations', to='socialaccount.SocialAccount', verbose_name='Connected account' ), diff --git a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py index ba69fca4f58..40fd61e7a55 100644 --- a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py @@ -10,10 +10,10 @@ log = logging.getLogger(__name__) -def move_data_to_remote_relations(apps, schema_editor): +def move_data_to_remote_repository_relations(apps, schema_editor): RemoteRepositoryRelation = apps.get_model('oauth', 'RemoteRepositoryRelation') - def remote_relations_generator(relations, batch_size): + def remote_repository_relations_generator(relations, batch_size): for relation in relations.iterator(chunk_size=batch_size): relation.account_id = relation.remoterepository.account_id relation.admin = relation.remoterepository.admin @@ -42,14 +42,14 @@ def remote_relations_generator(relations, batch_size): ) batch_size = 1000 - remote_relations = remote_relations_generator( + remote_repository_relations = remote_repository_relations_generator( relations_queryset, batch_size ) # Follows Example from django docs # https://docs.djangoproject.com/en/2.2/ref/models/querysets/#bulk-create while True: - batch = list(islice(remote_relations, batch_size)) + batch = list(islice(remote_repository_relations, batch_size)) if not batch: break @@ -72,5 +72,5 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(move_data_to_remote_relations), + migrations.RunPython(move_data_to_remote_repository_relations), ] diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 50b3240c734..a6c8957b474 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -206,18 +206,18 @@ def matches(self, user): class RemoteRepositoryRelation(TimeStampedModel): remoterepository = models.ForeignKey( RemoteRepository, - related_name='remote_relations', + related_name='remote_repository_relations', on_delete=models.CASCADE ) user = models.ForeignKey( User, - related_name='remote_relations', + related_name='remote_repository_relations', on_delete=models.CASCADE ) account = models.ForeignKey( SocialAccount, verbose_name=_('Connected account'), - related_name='remote_relations', + related_name='remote_repository_relations', null=True, blank=True, on_delete=models.SET_NULL, diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 76561520fd3..24ed875ab2a 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -202,7 +202,7 @@ def sync(self): all_remote_repositories = remote_repositories + remote_repositories_organizations repository_full_names = [r.full_name for r in all_remote_repositories if r is not None] ( - self.user.remote_relations + self.user.remote_repository_relations .exclude( Q(remoterepository__full_name__in=repository_full_names) | Q(remoterepository__project__isnull=False) diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index 0f93d0f0917..96a00b449cd 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -31,12 +31,12 @@ def update_webhook(project, integration, request=None): updated = False try: - remote_relations = project.remote_repository.remote_relations.filter( + remote_repository_relations = project.remote_repository.remote_repository_relations.filter( account__isnull=False, user=request.user ).select_related('account') - for relation in remote_relations: + for relation in remote_repository_relations: service = service_cls(request.user, relation.account) updated, __ = service.update_webhook(project, integration) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 1e890a5cfdc..194101b8fd2 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -970,7 +970,7 @@ def test_make_project_pass(self): ) self.assertEqual(repo.ssh_url, 'git@gitlab.com:testorga/testrepo.git') self.assertEqual(repo.html_url, 'https://gitlab.com/testorga/testrepo') - self.assertTrue(repo.remote_relations.first().admin) + self.assertTrue(repo.remote_repository_relations.first().admin) self.assertFalse(repo.private) def test_make_private_project_fail(self): From c194ac7e2b76f4a21fb66514772cafc9a36e14cb Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 21:13:38 +0600 Subject: [PATCH 19/22] Delete remote repository relation on sync without filtering with projects --- readthedocs/oauth/services/base.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 24ed875ab2a..89644ec793c 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -203,10 +203,7 @@ def sync(self): repository_full_names = [r.full_name for r in all_remote_repositories if r is not None] ( self.user.remote_repository_relations - .exclude( - Q(remoterepository__full_name__in=repository_full_names) | - Q(remoterepository__project__isnull=False) - ) + .exclude(remoterepository__full_name__in=repository_full_names) .filter(account=self.account) .delete() ) From 76eff0d2756a5a2d585e1d4234d35c0cd9f9bc17 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 22:27:10 +0600 Subject: [PATCH 20/22] Make remoterepository and account unique together --- .../oauth/migrations/0011_add_remote_relation_model.py | 10 +++++----- readthedocs/oauth/models.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py index 4b0f9ea4ed4..e7a44378bf2 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0011_add_remote_relation_model.py @@ -64,10 +64,6 @@ class Migration(migrations.Migration): verbose_name='Users' ), ), - migrations.AlterUniqueTogether( - name='remoterepositoryrelation', - unique_together={('remoterepository', 'user')}, - ), ], ), migrations.AddField( @@ -76,12 +72,16 @@ class Migration(migrations.Migration): field=models.ForeignKey( blank=True, null=True, - on_delete=django.db.models.deletion.SET_NULL, + on_delete=django.db.models.deletion.CASCADE, related_name='remote_repository_relations', to='socialaccount.SocialAccount', verbose_name='Connected account' ), ), + migrations.AlterUniqueTogether( + name='remoterepositoryrelation', + unique_together={('remoterepository', 'account')}, + ), migrations.AddField( model_name='remoterepositoryrelation', name='admin', diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index a6c8957b474..5c358bd87ea 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -220,13 +220,13 @@ class RemoteRepositoryRelation(TimeStampedModel): related_name='remote_repository_relations', null=True, blank=True, - on_delete=models.SET_NULL, + on_delete=models.CASCADE, ) admin = models.BooleanField(_('Has admin privilege'), default=False) json = JSONField(_('Serialized API response')) class Meta: - unique_together = (('remoterepository', 'user'),) + unique_together = (('remoterepository', 'account'),) def get_serialized(self, key=None, default=None): try: From 77c1777c51834ec6e552214d2508deb7dfda9b0d Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Tue, 24 Nov 2020 22:50:16 +0600 Subject: [PATCH 21/22] Fix sync_delete test --- readthedocs/rtd_tests/tests/test_oauth_sync.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py index 8ae8d02899f..08a9b6a3424 100644 --- a/readthedocs/rtd_tests/tests/test_oauth_sync.py +++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py @@ -98,7 +98,8 @@ def test_sync_delete_stale(self, mock_request): account=self.socialaccount ) - # RemoteRepository linked to a Project shouldn't be removed + # RemoteRepositoryRelation with RemoteRepository + # linked to a Project should also be removed project = fixture.get(Project) repo_3 = fixture.get( RemoteRepository, @@ -129,7 +130,7 @@ def test_sync_delete_stale(self, mock_request): # project-linked-repository remote relation should be conserved, # and only the one's returned by the API should be present (organization/repository) self.assertEqual(RemoteRepository.objects.count(), 3) - self.assertEqual(RemoteRepositoryRelation.objects.count(), 2) + self.assertEqual(RemoteRepositoryRelation.objects.count(), 1) self.assertTrue(RemoteRepository.objects.filter(full_name='organization/repository').exists()) self.assertTrue(RemoteRepository.objects.filter(full_name='organization/project-linked-repository').exists()) self.assertEqual(RemoteOrganization.objects.count(), 0) From bf8a1679c694ec0eba6788572d2aba4f307db97e Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Wed, 25 Nov 2020 21:40:19 +0600 Subject: [PATCH 22/22] Rename oauth migrations --- ...mote_relation_model.py => 0012_add_remote_relation_model.py} | 2 +- ...odel.py => 0013_data_migration_for_remote_relation_model.py} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename readthedocs/oauth/migrations/{0011_add_remote_relation_model.py => 0012_add_remote_relation_model.py} (98%) rename readthedocs/oauth/migrations/{0012_data_migration_for_remote_relation_model.py => 0013_data_migration_for_remote_relation_model.py} (97%) diff --git a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py b/readthedocs/oauth/migrations/0012_add_remote_relation_model.py similarity index 98% rename from readthedocs/oauth/migrations/0011_add_remote_relation_model.py rename to readthedocs/oauth/migrations/0012_add_remote_relation_model.py index e7a44378bf2..3ee1f737797 100644 --- a/readthedocs/oauth/migrations/0011_add_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0012_add_remote_relation_model.py @@ -12,7 +12,7 @@ class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('oauth', '0010_index_full_name'), + ('oauth', '0011_add_default_branch'), ] operations = [ diff --git a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py similarity index 97% rename from readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py rename to readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py index f28d9565df7..3ce92217e20 100644 --- a/readthedocs/oauth/migrations/0012_data_migration_for_remote_relation_model.py +++ b/readthedocs/oauth/migrations/0013_data_migration_for_remote_relation_model.py @@ -69,7 +69,7 @@ def remote_repository_relations_generator(relations, batch_size): class Migration(migrations.Migration): dependencies = [ - ('oauth', '0011_add_remote_relation_model'), + ('oauth', '0012_add_remote_relation_model'), ] operations = [