Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Build managers and Update Build Querysets. #5779

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
31b9bf2
Version Type Added
saadmk11 May 30, 2019
bd11994
Version Model Methods Updated
saadmk11 May 30, 2019
8f3e64b
Internal and External Version Manager added
saadmk11 Jun 4, 2019
66200a0
lint fix
saadmk11 Jun 4, 2019
34e4134
All Version Querysets Updated with InternalVersionManager
saadmk11 Jun 4, 2019
3a3cfdf
Manager names moved to Constants
saadmk11 Jun 5, 2019
3d3de29
Tests added
saadmk11 Jun 6, 2019
b55983f
Version Update, Delete Html and wipe updated to exclude PR Versions
saadmk11 Jun 6, 2019
6187272
Updated migrations
saadmk11 Jun 6, 2019
386ca07
footer API updated
saadmk11 Jun 8, 2019
29348ef
manager tests updated
saadmk11 Jun 11, 2019
11a0583
_override_setting removed from new managers
saadmk11 Jun 13, 2019
c100c81
BuildTriggerMixin updated
saadmk11 Jun 13, 2019
c1c03bb
More Update
saadmk11 Jun 13, 2019
5ea5f67
lint fix
saadmk11 Jun 13, 2019
73a46a5
Merge branch 'master' into version-update
saadmk11 Jun 13, 2019
56493fa
removed unused import
saadmk11 Jun 14, 2019
d8cff7e
Merge branch 'master' into version-update
saadmk11 Jun 17, 2019
84a6975
External version name added everywhere
saadmk11 Jun 17, 2019
05951cf
Migration name changed
saadmk11 Jun 17, 2019
e192300
Sitemap sort order priorities updated
saadmk11 May 23, 2019
d5ec837
ordering fix
saadmk11 May 25, 2019
1f13be1
doc string fix
saadmk11 May 25, 2019
e0232b3
sort_by_priority function added to sitemap
saadmk11 May 29, 2019
fdcfcf6
lint fix
saadmk11 May 29, 2019
ef13214
Sorting by swapping positions
saadmk11 Jun 11, 2019
736a337
index out of range issue fix
saadmk11 Jun 12, 2019
c18f381
Use a real SessionBase object on FooterNoSessionMiddleware
humitos Jun 12, 2019
908860a
Version Type Added
saadmk11 May 30, 2019
f81bcf0
Version Model Methods Updated
saadmk11 May 30, 2019
66377c2
Internal and External Version Manager added
saadmk11 Jun 4, 2019
8bdf75c
lint fix
saadmk11 Jun 4, 2019
21f96de
All Version Querysets Updated with InternalVersionManager
saadmk11 Jun 4, 2019
014477d
Manager names moved to Constants
saadmk11 Jun 5, 2019
d3f5254
Tests added
saadmk11 Jun 6, 2019
6bca1e4
Version Update, Delete Html and wipe updated to exclude PR Versions
saadmk11 Jun 6, 2019
8830a18
Updated migrations
saadmk11 Jun 6, 2019
5ed38c4
footer API updated
saadmk11 Jun 8, 2019
2d8b072
manager tests updated
saadmk11 Jun 11, 2019
89f3c8a
_override_setting removed from new managers
saadmk11 Jun 13, 2019
7b63973
BuildTriggerMixin updated
saadmk11 Jun 13, 2019
3a2fc6d
More Update
saadmk11 Jun 13, 2019
bca152a
lint fix
saadmk11 Jun 13, 2019
5f90052
Move search functions
stsewd Jun 13, 2019
120eb5d
removed unused import
saadmk11 Jun 14, 2019
ef3d061
External version name added everywhere
saadmk11 Jun 17, 2019
5b33bbd
Migration name changed
saadmk11 Jun 17, 2019
5180743
Build Managers added
saadmk11 Jun 8, 2019
8301679
Managers used in all the places
saadmk11 Jun 13, 2019
7b75ee8
build manager tests added
saadmk11 Jun 13, 2019
8c7b02b
More Tests added
saadmk11 Jun 13, 2019
5d6c750
External version name added
saadmk11 Jun 17, 2019
c5d1c09
Merge branch 'gsoc-19-pr-builder' into build-manager-update
saadmk11 Jun 17, 2019
7607268
Merge branch 'gsoc-19-pr-builder' into build-manager-update
saadmk11 Jun 18, 2019
75efef8
naming updated
saadmk11 Jun 18, 2019
51188e7
fix
saadmk11 Jun 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion readthedocs/api/v3/views.py
Expand Up @@ -269,7 +269,7 @@ class BuildsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin,
lookup_url_kwarg = 'build_pk'
serializer_class = BuildSerializer
filterset_class = BuildFilter
queryset = Build.objects.all()
queryset = Build.internal.all()
permit_list_expands = [
'config',
]
Expand Down
60 changes: 59 additions & 1 deletion readthedocs/builds/managers.py
Expand Up @@ -21,7 +21,7 @@
TAG,
EXTERNAL,
)
from .querysets import VersionQuerySet
from .querysets import VersionQuerySet, BuildQuerySet

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -122,3 +122,61 @@ class InternalVersionManager(SettingsOverrideObject):

class ExternalVersionManager(SettingsOverrideObject):
_default_class = ExternalVersionManagerBase


class BuildManagerBase(models.Manager):

"""
Build manager for manager only queries.

For creating different Managers.
"""

@classmethod
def from_queryset(cls, queryset_class, class_name=None):
# This is overridden because :py:meth:`models.Manager.from_queryset`
# uses `inspect` to retrieve the class methods, and the proxy class has
# no direct members.
queryset_class = get_override_class(
BuildQuerySet,
BuildQuerySet._default_class, # pylint: disable=protected-access
)
return super().from_queryset(queryset_class, class_name)


class InternalBuildManagerBase(BuildManagerBase):

"""
Build manager that only includes internal version builds.

It will exclude pull request/merge request version builds from the queries
and only include BRANCH, TAG, UNKONWN type Version builds.
"""

def get_queryset(self):
return super().get_queryset().exclude(version__type=EXTERNAL)


class ExternalBuildManagerBase(BuildManagerBase):

"""
Build manager that only includes external version builds.

It will only include pull request/merge request version builds in the queries.
"""

def get_queryset(self):
return super().get_queryset().filter(version__type=EXTERNAL)


class BuildManager(SettingsOverrideObject):
_default_class = BuildManagerBase
_override_setting = 'BUILD_MANAGER'


class InternalBuildManager(SettingsOverrideObject):
_default_class = InternalBuildManagerBase


class ExternalBuildManager(SettingsOverrideObject):
_default_class = ExternalBuildManagerBase
28 changes: 19 additions & 9 deletions readthedocs/builds/models.py
Expand Up @@ -46,18 +46,25 @@
TAG,
VERSION_TYPES,
)
from .managers import (
from readthedocs.builds.managers import (
VersionManager,
InternalVersionManager,
ExternalVersionManager
ExternalVersionManager,
BuildManager,
InternalBuildManager,
ExternalBuildManager,
)
from .querysets import BuildQuerySet, RelatedBuildQuerySet, VersionQuerySet
from .utils import (
from readthedocs.builds.querysets import (
BuildQuerySet,
RelatedBuildQuerySet,
VersionQuerySet,
)
from readthedocs.builds.utils import (
get_bitbucket_username_repo,
get_github_username_repo,
get_gitlab_username_repo,
)
from .version_slug import VersionSlugField
from readthedocs.builds.version_slug import VersionSlugField


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -200,7 +207,7 @@ def config(self):
:rtype: dict
"""
last_build = (
self.builds.filter(
self.builds(manager=INTERNAL).filter(
state='finished',
success=True,
).order_by('-date').first()
Expand Down Expand Up @@ -627,9 +634,12 @@ class Build(models.Model):
help_text='Build steps stored outside the database.',
)

# Manager

objects = BuildQuerySet.as_manager()
# Managers
objects = BuildManager.from_queryset(BuildQuerySet)()
# Only include BRANCH, TAG, UNKONWN type Version builds.
internal = InternalBuildManager.from_queryset(BuildQuerySet)()
# Only include EXTERNAL type Version builds.
external = ExternalBuildManager.from_queryset(BuildQuerySet)()

CONFIG_KEY = '__config'

Expand Down
1 change: 0 additions & 1 deletion readthedocs/builds/querysets.py
Expand Up @@ -116,7 +116,6 @@ def api(self, user=None, detail=True):

class BuildQuerySet(SettingsOverrideObject):
_default_class = BuildQuerySetBase
_override_setting = 'BUILD_MANAGER'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't change this setting. We need to be able to still override this.

Copy link
Member Author

@saadmk11 saadmk11 Jun 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericholscher actually we are now using BuildManager not BuildQueryset.as_manager() so i put this settings in the BuildManager.
https://github.com/rtfd/readthedocs.org/blob/3f93788ea69ad90c6230f4e9d19aae43a8428020/readthedocs/builds/managers.py#L176



class RelatedBuildQuerySetBase(models.QuerySet):
Expand Down
15 changes: 15 additions & 0 deletions readthedocs/builds/views.py
Expand Up @@ -84,6 +84,21 @@ def post(self, request, project_slug):

class BuildList(BuildBase, BuildTriggerMixin, ListView):

def get_queryset(self):
# this is used to include only internal version
# builds in the build list page
self.project_slug = self.kwargs.get('project_slug', None)
self.project = get_object_or_404(
Project.objects.protected(self.request.user),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't used protected, let's just use public.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericholscher This is default in the base class should we change this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericholscher This is default in the base class and we were using it should we change it?

slug=self.project_slug,
)
queryset = Build.internal.public(
user=self.request.user,
project=self.project,
).select_related('project', 'version')

return queryset

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/templatetags/privacy_tags.py
Expand Up @@ -26,7 +26,7 @@ def get_public_projects(context, user):
viewer=context['request'].user,
).prefetch_latest_build().annotate(
_good_build=Exists(
Build.objects.filter(success=True, project=OuterRef('pk')))
Build.internal.filter(success=True, project=OuterRef('pk')))
)
context['public_projects'] = projects
return ''
2 changes: 1 addition & 1 deletion readthedocs/projects/models.py
Expand Up @@ -768,7 +768,7 @@ def has_good_build(self):
# Used for Database optimization.
if hasattr(self, '_good_build'):
return self._good_build
return self.builds.filter(success=True).exists()
return self.builds(manager=INTERNAL).filter(success=True).exists()

@property
def has_versions(self):
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/projects/querysets.py
Expand Up @@ -85,13 +85,13 @@ def prefetch_latest_build(self):

# Prefetch the latest build for each project.
subquery = Subquery(
Build.objects.filter(
Build.internal.filter(
project=OuterRef('project_id')
).order_by('-date').values_list('id', flat=True)[:1]
)
latest_build = Prefetch(
'builds',
Build.objects.filter(pk__in=subquery),
Build.internal.filter(pk__in=subquery),
to_attr=self.model.LATEST_BUILD_CACHE,
)
return self.prefetch_related(latest_build)
Expand Down
3 changes: 2 additions & 1 deletion readthedocs/rtd_tests/tests/test_doc_serving.py
Expand Up @@ -313,7 +313,7 @@ def test_sitemap_xml(self):
# in language and country value. (zh_CN should be zh-CN)
self.assertContains(response, 'zh-CN')

# PR Versions should not be in the sitemap_xml.
# External Versions should not be in the sitemap_xml.
self.assertNotContains(
response,
self.public.get_docs_url(
Expand All @@ -322,6 +322,7 @@ def test_sitemap_xml(self):
private=True,
),
)

# Check if STABLE version has 'priority of 1 and changefreq of weekly.
self.assertEqual(
response.context['versions'][0]['loc'],
Expand Down
90 changes: 89 additions & 1 deletion readthedocs/rtd_tests/tests/test_managers.py
Expand Up @@ -3,7 +3,7 @@
from django_dynamic_fixture import get

from readthedocs.builds.constants import EXTERNAL, BRANCH, TAG
from readthedocs.builds.models import Version
from readthedocs.builds.models import Version, Build
from readthedocs.projects.constants import PUBLIC, PRIVATE, PROTECTED
from readthedocs.projects.models import Project

Expand Down Expand Up @@ -123,3 +123,91 @@ def test_external_version_manager_with_for_project(self):
self.assertIn(
self.public_external_version, Version.external.for_project(self.pip)
)


class TestBuildManagerBase(TestCase):

fixtures = ['test_data']

def setUp(self):
self.user = User.objects.create(username='test_user', password='test')
self.client.login(username='test_user', password='test')
self.pip = Project.objects.get(slug='pip')
print(self.pip.versions.all())
# Create a External Version and build. ie: pull/merge request Version.
self.external_version = get(
Version,
project=self.pip,
active=True,
type=EXTERNAL,
privacy_level=PUBLIC
)
self.external_version_build = get(
Build,
project=self.pip,
version=self.external_version
)
# Create a Internal Version build.
self.internal_version_build = get(
Build,
project=self.pip,
version=self.pip.versions.get(slug='0.8')
)

self.internal_builds = Build.objects.exclude(version__type=EXTERNAL)


class TestInternalBuildManager(TestBuildManagerBase):

"""
Queries using Internal Manager should only include Internal Version builds.

It will exclude pull/merge request Version builds from the queries
and only include BRANCH, TAG, UNKONWN type Versions.
"""

def test_internal_build_manager_with_all(self):
self.assertNotIn(self.external_version_build, Build.internal.all())

def test_internal_build_manager_with_public(self):
self.assertNotIn(self.external_version_build, Build.internal.public())

def test_internal_build_manager_with_public_with_user_and_project(self):
self.assertNotIn(
self.external_version_build,
Build.internal.public(self.user, self.pip)
)

def test_internal_build_manager_with_api(self):
self.assertNotIn(self.external_version_build, Build.internal.api())


class TestExternalBuildManager(TestBuildManagerBase):

"""
Queries using External Manager should only include External Version builds.

It will only include pull/merge request Version builds in the queries.
"""

def test_external_build_manager_with_all(self):
self.assertNotIn(self.internal_builds, Build.external.all())
self.assertIn(self.external_version_build, Build.external.all())

def test_external_build_manager_with_public(self):
self.assertNotIn(self.internal_builds, Build.external.public())
self.assertIn(self.external_version_build, Build.external.public())

def test_external_build_manager_with_public_with_user_and_project(self):
self.assertNotIn(
self.internal_builds,
Build.external.public(self.user, self.pip)
)
self.assertIn(
self.external_version_build,
Build.external.public(self.user, self.pip)
)

def test_external_build_manager_with_api(self):
self.assertNotIn(self.internal_builds, Build.external.api())
self.assertIn(self.external_version_build, Build.external.api())
6 changes: 6 additions & 0 deletions readthedocs/rtd_tests/tests/test_project.py
Expand Up @@ -160,6 +160,12 @@ def test_update_stable_version_excludes_external_versions(self):
# Test that PR Version is not considered for stable.
self.assertEqual(self.pip.update_stable_version(), None)

def test_has_good_build_excludes_external_versions(self):
# Delete all versions excluding PR Versions.
self.pip.versions.exclude(type=EXTERNAL).delete()
# Test that PR Version is not considered for has_good_build.
self.assertFalse(self.pip.has_good_build)


class TestProjectTranslations(ProjectMixin, TestCase):

Expand Down
25 changes: 23 additions & 2 deletions readthedocs/rtd_tests/tests/test_views.py
Expand Up @@ -7,8 +7,8 @@
from django.urls import reverse
from django_dynamic_fixture import get, new

from readthedocs.builds.constants import LATEST
from readthedocs.builds.models import Build
from readthedocs.builds.constants import LATEST, EXTERNAL
from readthedocs.builds.models import Build, Version
from readthedocs.core.permissions import AdminPermission
from readthedocs.projects.forms import UpdateProjectForm
from readthedocs.projects.models import HTMLFile, Project
Expand Down Expand Up @@ -266,6 +266,7 @@ class BuildViewTests(TestCase):

def setUp(self):
self.client.login(username='eric', password='test')
self.pip = Project.objects.get(slug='pip')

@mock.patch('readthedocs.projects.tasks.update_docs_task')
def test_build_redirect(self, mock):
Expand All @@ -276,3 +277,23 @@ def test_build_redirect(self, mock):
r._headers['location'][1],
'/projects/pip/builds/%s/' % build.pk,
)

def test_build_list_does_not_include_external_versions(self):
external_version = get(
Version,
project = self.pip,
active = True,
type = EXTERNAL,
)
external_version_build = get(
Build,
project = self.pip,
version = external_version
)
response = self.client.get(
reverse('builds_project_list', args=[self.pip.slug]),
)
self.assertEqual(response.status_code, 200)

self.assertNotIn(external_version_build, response.context['build_qs'])
self.assertNotIn(external_version_build, response.context['active_builds'])