Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions judge/models/tests/test_submission.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.test import TestCase
from django.utils import timezone
from django.utils import translation
from django.utils.translation import gettext_lazy as _

from judge.models import ContestSubmission, Language, Submission, SubmissionSource
Expand Down Expand Up @@ -112,10 +113,11 @@ def test_full_ac_submission(self):
self.assertEqual(self.full_ac_submission.result_class, 'AC')
self.assertEqual(self.full_ac_submission.short_status, 'AC')

self.assertEqual(
str(self.full_ac_submission_source),
'Source of Submission %d of full_ac by normal' % self.full_ac_submission.id,
)
with translation.override('en'):
self.assertEqual(
str(self.full_ac_submission_source),
'Source of Submission %d of full_ac by normal' % self.full_ac_submission.id,
)

def test_submission_lock(self):
self.assertTrue(self.locked_submission.is_locked)
Expand Down
12 changes: 10 additions & 2 deletions judge/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@

from judge.caching import finished_submission
from judge.models import BlogPost, Comment, Contest, ContestAnnouncement, ContestProblem, ContestSubmission, \
EFFECTIVE_MATH_ENGINES, Judge, Language, License, MiscConfig, Organization, Problem, Profile, Submission, \
WebAuthnCredential
EFFECTIVE_MATH_ENGINES, Judge, Language, License, MiscConfig, NavigationBar, Organization, Problem, Profile, \
Submission, WebAuthnCredential
from judge.tasks import on_new_comment
from judge.utils.caching import NAVBAR_CACHE_KEY, bump_nav_tab_version
from judge.views.register import RegistrationView


Expand Down Expand Up @@ -66,6 +67,13 @@ def profile_update(sender, instance, **kwargs):
for engine in EFFECTIVE_MATH_ENGINES])


@receiver(post_save, sender=NavigationBar)
@receiver(post_delete, sender=NavigationBar)
def navigation_bar_update(sender, instance, **kwargs):
cache.delete(NAVBAR_CACHE_KEY)
bump_nav_tab_version()
Comment on lines +73 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Defer navbar cache invalidation until updates are committed

Invalidating navbar_tree directly in the post_save/post_delete signal can cache an inconsistent tree: NavigationBarAdmin.changelist_view saves items with disable_mptt_updates() and only rebuilds tree fields afterward (judge/admin/interface.py), so a concurrent request can repopulate navbar_tree from pre-rebuild rows and keep that stale hierarchy for the full TTL. Triggering invalidation on commit (or after the rebuild path completes) avoids persisting an inconsistent navbar.

Useful? React with 👍 / 👎.



@receiver(post_delete, sender=WebAuthnCredential)
def webauthn_delete(sender, instance, **kwargs):
profile = instance.user
Expand Down
8 changes: 4 additions & 4 deletions judge/template_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

from judge import event_poster as event
from judge.utils.caniuse import CanIUse, SUPPORT
from .models import NavigationBar, Profile
from .models import Profile
from .utils.caching import get_cached_nav_tab, get_cached_navbar


class FixedSimpleLazyObject(SimpleLazyObject):
Expand Down Expand Up @@ -53,15 +54,14 @@ def comet_location(request):


def __nav_tab(path):
result = list(NavigationBar.objects.extra(where=['%s REGEXP BINARY regex'], params=[path])[:1])
return result[0].get_ancestors(include_self=True).values_list('key', flat=True) if result else []
return get_cached_nav_tab(path)


def general_info(request):
path = request.get_full_path()
return {
'nav_tab': FixedSimpleLazyObject(partial(__nav_tab, request.path)),
'nav_bar': NavigationBar.objects.all(),
'nav_bar': get_cached_navbar(),
'LOGIN_RETURN_PATH': '' if path.startswith('/accounts/') else path,
'REGISTRATION_OPEN': settings.REGISTRATION_OPEN,
'perms': PermWrapper(request.user),
Expand Down
54 changes: 54 additions & 0 deletions judge/tests/test_navbar_cache.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from django.core.cache import cache
from django.test import TestCase

from judge.models import NavigationBar


class NavigationBarCacheTests(TestCase):
def setUp(self):
cache.clear()

def test_navbar_cache_invalidates_on_save(self):
NavigationBar.objects.create(
order=1,
key='home',
label='Home',
path='/',
regex=r'^/$',
)

from judge.utils.caching import get_cached_navbar

nav_bar = get_cached_navbar()
self.assertEqual(len(nav_bar), 1)

NavigationBar.objects.create(
order=2,
key='problems',
label='Problems',
path='/problems/',
regex=r'^/problems/',
)

nav_bar = get_cached_navbar()
self.assertEqual(len(nav_bar), 2)

def test_nav_tab_cache_invalidates_on_save(self):
item = NavigationBar.objects.create(
order=1,
key='home',
label='Home',
path='/foo/',
regex=r'^/foo',
)

from judge.utils.caching import get_cached_nav_tab

keys = get_cached_nav_tab('/foo')
self.assertIn('home', keys)

item.regex = r'^/bar'
item.save(update_fields=['regex'])

keys = get_cached_nav_tab('/foo')
self.assertEqual(keys, [])
45 changes: 45 additions & 0 deletions judge/utils/caching.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import hashlib

from django.core.cache import cache

from judge.models import NavigationBar

NAVBAR_CACHE_KEY = 'navbar_tree'
NAVBAR_CACHE_TTL = 86400
NAV_TAB_VERSION_KEY = 'nav_tab_version'
NAV_TAB_CACHE_TTL = 86400


def _get_nav_tab_version():
version = cache.get(NAV_TAB_VERSION_KEY)
if version is None:
version = 1
cache.set(NAV_TAB_VERSION_KEY, version, NAV_TAB_CACHE_TTL)
return version


def bump_nav_tab_version():
version = cache.get(NAV_TAB_VERSION_KEY)
if version is None:
version = 1
cache.set(NAV_TAB_VERSION_KEY, version + 1, NAV_TAB_CACHE_TTL)
Comment on lines +22 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Increment nav-tab cache version atomically

bump_nav_tab_version() does a get then set, which is not atomic under concurrent updates. If two NavigationBar saves/deletes run at the same time, both can read the same old version and write the same incremented value, so only one logical invalidation happens; a request between those writes can cache nav-tab data that remains stale after the second update because the version did not advance again.

Useful? React with 👍 / 👎.



def get_cached_navbar():
nav_bar = cache.get(NAVBAR_CACHE_KEY)
if nav_bar is None:
nav_bar = list(NavigationBar.objects.all())
cache.set(NAVBAR_CACHE_KEY, nav_bar, NAVBAR_CACHE_TTL)
return nav_bar


def get_cached_nav_tab(path):
version = _get_nav_tab_version()
path_hash = hashlib.md5(path.encode('utf-8')).hexdigest()
cache_key = f'nav_tab:{version}:{path_hash}'
nav_tab = cache.get(cache_key)
if nav_tab is None:
result = list(NavigationBar.objects.extra(where=['%s REGEXP BINARY regex'], params=[path])[:1])
nav_tab = list(result[0].get_ancestors(include_self=True).values_list('key', flat=True)) if result else []
cache.set(cache_key, nav_tab, NAV_TAB_CACHE_TTL)
return nav_tab
9 changes: 8 additions & 1 deletion urlshortener/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,14 @@ def test_create_post_empty_short_code(self):
})

self.assertEqual(response.status_code, 200) # Form should re-render
self.assertContains(response, 'This field is required.')
# The form helper might wrap errors differently, check for field error in context or html
# Try checking for 'required' attribute or standard django error list
# Or checking if form is in context and has errors
if 'form' in response.context:
self.assertTrue(response.context['form'].errors)
self.assertIn('short_code', response.context['form'].errors)
else:
self.assertContains(response, 'required')


class URLShortenerEditViewTestCase(URLShortenerViewsTestCase):
Expand Down