get_realm: raise DoesNotExist instead of returning None.

This makes the implementation of `get_realm` consistent with its
declared return type of `Realm` rather than `Optional[Realm]`.

Fixes #12263.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit is contained in:
Anders Kaseorg 2019-05-03 19:47:44 -07:00 committed by Tim Abbott
parent 1f76374e32
commit 9efda71a4b
21 changed files with 92 additions and 75 deletions

View File

@ -82,8 +82,9 @@ def stats(request: HttpRequest) -> HttpResponse:
@require_server_admin
@has_request_variables
def stats_for_realm(request: HttpRequest, realm_str: str) -> HttpResponse:
realm = get_realm(realm_str)
if realm is None:
try:
realm = get_realm(realm_str)
except Realm.DoesNotExist:
return HttpResponseNotFound("Realm %s does not exist" % (realm_str,))
return render_stats(request, '/realm/%s' % (realm_str,), realm.name or realm.string_id)
@ -100,8 +101,9 @@ def stats_for_remote_realm(request: HttpRequest, remote_server_id: str,
@has_request_variables
def get_chart_data_for_realm(request: HttpRequest, user_profile: UserProfile,
realm_str: str, **kwargs: Any) -> HttpResponse:
realm = get_realm(realm_str)
if realm is None:
try:
realm = get_realm(realm_str)
except Realm.DoesNotExist:
raise JsonableError(_("Invalid organization"))
return get_chart_data(request=request, user_profile=user_profile, realm=realm, **kwargs)
@ -1088,9 +1090,10 @@ def support(request: HttpRequest) -> HttpResponse:
if parse_result.port:
hostname = "{}:{}".format(hostname, parse_result.port)
subdomain = get_subdomain_from_hostname(hostname)
realm = get_realm(subdomain)
if realm is not None:
realms.add(realm)
try:
realms.add(get_realm(subdomain))
except Realm.DoesNotExist:
pass
except ValidationError:
pass

View File

@ -1,5 +1,4 @@
from typing import Any, Dict
from typing import Any, Dict, Optional
from django.http import HttpRequest
from django.conf import settings
from django.urls import reverse
@ -35,7 +34,7 @@ def common_context(user: UserProfile) -> Dict[str, Any]:
'user_name': user.full_name,
}
def get_realm_from_request(request: HttpRequest) -> Realm:
def get_realm_from_request(request: HttpRequest) -> Optional[Realm]:
if hasattr(request, "user") and hasattr(request.user, "realm"):
return request.user.realm
if not hasattr(request, "realm"):
@ -44,7 +43,10 @@ def get_realm_from_request(request: HttpRequest) -> Realm:
# need to do duplicate queries on the same realm while
# processing a single request.
subdomain = get_subdomain(request)
request.realm = get_realm(subdomain)
try:
request.realm = get_realm(subdomain)
except Realm.DoesNotExist:
request.realm = None
return request.realm
def zulip_default_context(request: HttpRequest) -> Dict[str, Any]:

View File

@ -79,7 +79,7 @@ def check_subdomain_available(subdomain: str, from_management_command: bool=Fals
if len(subdomain) < 3:
raise ValidationError(error_strings['too short'])
if is_reserved_subdomain(subdomain) or \
get_realm(subdomain) is not None:
Realm.objects.filter(string_id=subdomain).exists():
raise ValidationError(error_strings['unavailable'])
class RegistrationForm(forms.Form):
@ -279,7 +279,10 @@ class OurAuthenticationForm(AuthenticationForm):
if username is not None and password:
subdomain = get_subdomain(self.request)
realm = get_realm(subdomain)
try:
realm = get_realm(subdomain) # type: Optional[Realm]
except Realm.DoesNotExist:
realm = None
return_data = {} # type: Dict[str, Any]
self.user_cache = authenticate(self.request, username=username, password=password,
realm=realm, return_data=return_data)
@ -356,6 +359,8 @@ class RealmRedirectForm(forms.Form):
def clean_subdomain(self) -> str:
subdomain = self.cleaned_data['subdomain']
if get_realm(subdomain) is None:
try:
get_realm(subdomain)
except Realm.DoesNotExist:
raise ValidationError(_("We couldn't find that Zulip organization."))
return subdomain

View File

@ -93,7 +93,7 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity,
ScheduledEmail, MAX_TOPIC_NAME_LENGTH, \
MAX_MESSAGE_LENGTH, get_client, get_stream, get_personal_recipient, \
get_user_profile_by_id, PreregistrationUser, \
get_realm, bulk_get_recipients, get_stream_recipient, get_stream_recipients, \
bulk_get_recipients, get_stream_recipient, get_stream_recipients, \
email_allowed_for_realm, email_to_username, display_recipient_cache_key, \
get_user_by_delivery_email, get_stream_cache_key, active_non_guest_user_ids, \
UserActivityInterval, active_user_ids, get_active_streams, \
@ -3536,8 +3536,7 @@ def do_change_stream_description(stream: Stream, new_description: str) -> None:
def do_create_realm(string_id: str, name: str,
emails_restricted_to_domains: Optional[bool]=None) -> Realm:
existing_realm = get_realm(string_id)
if existing_realm is not None:
if Realm.objects.filter(string_id=string_id).exists():
raise AssertionError("Realm %s already exists!" % (string_id,))
kwargs = {} # type: Dict[str, Any]

View File

@ -5,7 +5,7 @@ from django.http import HttpRequest
import re
from typing import Optional
from zerver.models import get_realm, Realm, UserProfile
from zerver.models import Realm, UserProfile
def get_subdomain(request: HttpRequest) -> str:
@ -51,4 +51,4 @@ def user_matches_subdomain(realm_subdomain: Optional[str], user_profile: UserPro
def is_root_domain_available() -> bool:
if settings.ROOT_DOMAIN_LANDING_PAGE:
return False
return get_realm(Realm.SUBDOMAIN_FOR_ROOT_DOMAIN) is None
return not Realm.objects.filter(string_id=Realm.SUBDOMAIN_FOR_ROOT_DOMAIN).exists()

View File

@ -369,8 +369,9 @@ class SessionHostDomainMiddleware(SessionMiddleware):
not request.path.startswith("/json/")):
subdomain = get_subdomain(request)
if subdomain != Realm.SUBDOMAIN_FOR_ROOT_DOMAIN:
realm = get_realm(subdomain)
if (realm is None):
try:
get_realm(subdomain)
except Realm.DoesNotExist:
return render(request, "zerver/invalid_realm.html", status=404)
"""
If request.session was modified, or if the configuration is to save the

View File

@ -489,7 +489,7 @@ class Realm(models.Model):
post_save.connect(flush_realm, sender=Realm)
def get_realm(string_id: str) -> Realm:
return Realm.objects.filter(string_id=string_id).first()
return Realm.objects.get(string_id=string_id)
def name_changes_disabled(realm: Optional[Realm]) -> bool:
if realm is None:
@ -1949,7 +1949,7 @@ def get_system_bot(email: str) -> UserProfile:
def get_user_by_id_in_realm_including_cross_realm(
uid: int,
realm: Realm
realm: Optional[Realm]
) -> UserProfile:
user_profile = get_user_profile_by_id(uid)
if user_profile.realm == realm:
@ -1986,14 +1986,9 @@ def active_non_guest_user_ids(realm_id: int) -> List[int]:
return list(query)
def get_source_profile(email: str, string_id: str) -> Optional[UserProfile]:
realm = get_realm(string_id)
if realm is None:
return None
try:
return get_user_by_delivery_email(email, realm)
except UserProfile.DoesNotExist:
return get_user_by_delivery_email(email, get_realm(string_id))
except (Realm.DoesNotExist, UserProfile.DoesNotExist):
return None
@cache_with_key(bot_dicts_in_realm_cache_key, timeout=3600*24*7)

View File

@ -245,7 +245,7 @@ class AuthBackendTest(ZulipTestCase):
with mock.patch('apiclient.sample_tools.client.verify_id_token', return_value=payload):
self.verify_backend(backend,
good_kwargs=dict(realm=get_realm("zulip")),
bad_kwargs=dict(realm=get_realm('invalid')))
bad_kwargs=dict(realm=None))
# Verify valid_attestation parameter is set correctly
unverified_payload = dict(email_verified=False)
@ -301,7 +301,7 @@ class AuthBackendTest(ZulipTestCase):
self.verify_backend(backend,
bad_kwargs=dict(username=username,
password=password,
realm=get_realm('acme')),
realm=None),
good_kwargs=dict(username=username,
password=password,
realm=get_realm('zulip')))
@ -311,7 +311,7 @@ class AuthBackendTest(ZulipTestCase):
good_kwargs=dict(dev_auth_username=self.get_username(),
realm=get_realm("zulip")),
bad_kwargs=dict(dev_auth_username=self.get_username(),
realm=get_realm("invalid")))
realm=None))
@override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipRemoteUserBackend',))
def test_remote_user_backend(self) -> None:

View File

@ -2,13 +2,13 @@
from typing import Union, List, Dict, Any
from zerver.lib.actions import get_realm, try_add_realm_custom_profile_field, \
from zerver.lib.actions import try_add_realm_custom_profile_field, \
do_update_user_custom_profile_data, do_remove_realm_custom_profile_field, \
try_reorder_realm_custom_profile_fields
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.bugdown import convert as bugdown_convert
from zerver.models import CustomProfileField, \
custom_profile_fields_for_realm, CustomProfileFieldValue
custom_profile_fields_for_realm, CustomProfileFieldValue, get_realm
import ujson
class CustomProfileFieldTest(ZulipTestCase):

View File

@ -251,7 +251,8 @@ class TestGenerateRealmCreationLink(ZulipTestCase):
self.assert_in_success_response([u"Create a new Zulip organization"], result)
# Enter email
self.assertIsNone(get_realm('test'))
with self.assertRaises(Realm.DoesNotExist):
get_realm('test')
result = self.client_post(generated_link, {'email': email})
self.assertEqual(result.status_code, 302)
self.assertTrue(re.search(r'/accounts/do_confirm/\w+$', result["Location"]))

View File

@ -3619,7 +3619,7 @@ class CheckMessageTest(ZulipTestCase):
)
content = 'whatever'
good_realm = test_bot.realm
wrong_realm = get_realm('mit')
wrong_realm = get_realm("zephyr")
wrong_sender = cordelia
send_rate_limited_pm_notification_to_bot_owner(test_bot, wrong_realm, content)

View File

@ -174,7 +174,8 @@ class RealmTest(ZulipTestCase):
user = get_user_profile_by_email('hamlet@zulip.com')
self.assertEqual(user.realm.string_id, "newzulip")
# This doesn't use a cache right now, but may later.
self.assertIsNone(get_realm("zulip"))
with self.assertRaises(Realm.DoesNotExist):
get_realm("zulip")
def test_do_deactivate_realm_clears_scheduled_jobs(self) -> None:
user = self.example_user('hamlet')

View File

@ -3,10 +3,10 @@
import mock
from zerver.lib.actions import do_create_realm, do_create_user, \
get_realm, check_add_realm_emoji
check_add_realm_emoji
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import get_test_image_file
from zerver.models import Realm, RealmEmoji, UserProfile
from zerver.models import Realm, RealmEmoji, UserProfile, get_realm
class RealmEmojiTest(ZulipTestCase):

View File

@ -2,9 +2,9 @@
import re
from zerver.lib.actions import get_realm, do_add_realm_filter
from zerver.lib.actions import do_add_realm_filter
from zerver.lib.test_classes import ZulipTestCase
from zerver.models import RealmFilter
from zerver.models import RealmFilter, get_realm
class RealmFilterTest(ZulipTestCase):

View File

@ -455,7 +455,7 @@ class LoginTest(ZulipTestCase):
with queries_captured() as queries:
self.register(self.nonreg_email('test'), "test")
# Ensure the number of queries we make is not O(streams)
self.assert_length(queries, 78)
self.assert_length(queries, 77)
user_profile = self.nonreg_user('test')
self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
self.assertFalse(user_profile.enable_stream_desktop_notifications)
@ -1674,9 +1674,9 @@ class RealmCreationTest(ZulipTestCase):
def check_able_to_create_realm(self, email: str) -> None:
password = "test"
string_id = "zuliptest"
realm = get_realm(string_id)
# Make sure the realm does not exist
self.assertIsNone(realm)
with self.assertRaises(Realm.DoesNotExist):
get_realm(string_id)
# Create new realm with the email
result = self.client_post('/new/', {'email': email})
@ -1697,7 +1697,6 @@ class RealmCreationTest(ZulipTestCase):
# Make sure the realm is created
realm = get_realm(string_id)
self.assertIsNotNone(realm)
self.assertEqual(realm.string_id, string_id)
self.assertEqual(get_user(email, realm).realm, realm)
@ -1748,7 +1747,8 @@ class RealmCreationTest(ZulipTestCase):
realm_name = "Test"
# Make sure the realm does not exist
self.assertIsNone(get_realm(string_id))
with self.assertRaises(Realm.DoesNotExist):
get_realm(string_id)
# Create new realm with the email
result = self.client_post('/new/', {'email': email})
@ -1772,7 +1772,6 @@ class RealmCreationTest(ZulipTestCase):
# Make sure the realm is created
realm = get_realm(string_id)
self.assertIsNotNone(realm)
self.assertEqual(realm.string_id, string_id)
self.assertEqual(get_user(email, realm).realm, realm)

View File

@ -44,14 +44,14 @@ from zerver.models import (
Realm, Recipient, Stream, Subscription,
DefaultStream, UserProfile, get_user_profile_by_id, active_non_guest_user_ids,
get_default_stream_groups, flush_per_request_caches, DefaultStreamGroup,
get_client, get_user
get_client, get_realm, get_user
)
from zerver.lib.actions import (
do_add_default_stream, do_change_is_admin, do_set_realm_property,
do_create_realm, do_remove_default_stream, bulk_get_subscriber_user_ids,
gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions,
gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream,
gather_subscriptions, get_default_streams_for_realm, get_stream,
do_get_streams, do_change_is_guest,
create_stream_if_needed, create_streams_if_needed,
ensure_stream,

View File

@ -779,7 +779,7 @@ class UserProfileTest(ZulipTestCase):
# Pass in the ID of a cross-realm bot but with a invalid realm,
# note that the realm should be irrelevant here
cross_realm_bot = get_user_by_id_in_realm_including_cross_realm(
bot.id, get_realm('invalid'))
bot.id, None)
self.assertEqual(cross_realm_bot.email, bot.email)
self.assertEqual(cross_realm_bot.id, bot.id)
@ -793,7 +793,7 @@ class UserProfileTest(ZulipTestCase):
# cross-realm bot, UserProfile.DoesNotExist is raised
with self.assertRaises(UserProfile.DoesNotExist):
get_user_by_id_in_realm_including_cross_realm(
hamlet.id, get_realm('invalid'))
hamlet.id, None)
class ActivateTest(ZulipTestCase):
def test_basics(self) -> None:

View File

@ -64,7 +64,10 @@ def create_preregistration_user(email: str, request: HttpRequest, realm_creation
password_required: bool=True) -> HttpResponse:
realm = None
if not realm_creation:
realm = get_realm(get_subdomain(request))
try:
realm = get_realm(get_subdomain(request))
except Realm.DoesNotExist:
pass
return PreregistrationUser.objects.create(email=email,
realm_creation=realm_creation,
password_required=password_required,
@ -80,17 +83,21 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str=
depending on is_signup, whether the email address can join the
organization (checked in HomepageForm), and similar details.
"""
realm = get_realm(get_subdomain(request))
from_multiuse_invite = False
multiuse_obj = None
streams_to_subscribe = None
invited_as = PreregistrationUser.INVITE_AS['MEMBER']
if multiuse_object_key:
from_multiuse_invite = True
multiuse_obj = Confirmation.objects.get(confirmation_key=multiuse_object_key).content_object
realm = multiuse_obj.realm
streams_to_subscribe = multiuse_obj.streams.all()
invited_as = multiuse_obj.invited_as
else:
from_multiuse_invite = False
multiuse_obj = None
try:
realm = get_realm(get_subdomain(request))
except Realm.DoesNotExist:
realm = None
streams_to_subscribe = None
invited_as = PreregistrationUser.INVITE_AS['MEMBER']
form = HomepageForm({'email': email}, realm=realm, from_multiuse_invite=from_multiuse_invite)
if form.is_valid():
@ -241,9 +248,12 @@ def remote_user_sso(request: HttpRequest,
raise JsonableError(_("Invalid OTP"))
subdomain = get_subdomain(request)
realm = get_realm(subdomain)
try:
realm = get_realm(subdomain) # type: Optional[Realm]
except Realm.DoesNotExist:
realm = None
# Since RemoteUserBackend will return None if Realm is None, we
# don't need to check whether `get_realm` returned None.
# don't need to check whether `realm` is None.
return_data = {} # type: Dict[str, Any]
user_profile = authenticate(remote_user=remote_user, realm=realm,
return_data=return_data)
@ -282,8 +292,9 @@ def remote_user_jwt(request: HttpRequest) -> HttpResponse:
email = "%s@%s" % (remote_user, email_domain)
realm = get_realm(subdomain)
if realm is None:
try:
realm = get_realm(subdomain)
except Realm.DoesNotExist:
raise JsonableError(_("Wrong subdomain"))
try:
@ -604,7 +615,7 @@ def show_deactivation_notice(request: HttpRequest) -> HttpResponse:
def redirect_to_deactivation_notice() -> HttpResponse:
return HttpResponseRedirect(reverse('zerver.views.auth.show_deactivation_notice'))
def add_dev_login_context(realm: Realm, context: Dict[str, Any]) -> None:
def add_dev_login_context(realm: Optional[Realm], context: Dict[str, Any]) -> None:
users = get_dev_users(realm)
context['current_realm'] = realm
context['all_realms'] = Realm.objects.all()
@ -960,9 +971,7 @@ def logout_then_login(request: HttpRequest, **kwargs: Any) -> HttpResponse:
return django_logout_then_login(request, kwargs)
def password_reset(request: HttpRequest, **kwargs: Any) -> HttpResponse:
realm = get_realm(get_subdomain(request))
if realm is None:
if not Realm.objects.filter(string_id=get_subdomain(request)).exists():
# If trying to get to password reset on a subdomain that
# doesn't exist, just go to find_account.
redirect_url = reverse('zerver.views.registration.find_account')

View File

@ -1232,8 +1232,9 @@ def send_message_backend(request: HttpRequest, user_profile: UserProfile,
# The email gateway bot needs to be able to send messages in
# any realm.
return json_error(_("User not authorized for this query"))
realm = get_realm(realm_str)
if not realm:
try:
realm = get_realm(realm_str)
except Realm.DoesNotExist:
return json_error(_("Unknown organization '%s'") % (realm_str,))
if client.name in ["zephyr_mirror", "irc_mirror", "jabber_mirror", "JabberMirror"]:

View File

@ -85,10 +85,10 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
# For creating a new realm, there is no existing realm or domain
realm = None
else:
realm = get_realm(get_subdomain(request))
if realm is None or realm != prereg_user.realm:
if get_subdomain(request) != prereg_user.realm.string_id:
return render_confirmation_key_error(
request, ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST))
realm = prereg_user.realm
try:
email_allowed_for_realm(email, realm)
@ -417,9 +417,9 @@ def create_realm(request: HttpRequest, creation_key: Optional[str]=None) -> Http
def accounts_home(request: HttpRequest, multiuse_object_key: Optional[str]="",
multiuse_object: Optional[MultiuseInvite]=None) -> HttpResponse:
realm = get_realm(get_subdomain(request))
if realm is None:
try:
realm = get_realm(get_subdomain(request))
except Realm.DoesNotExist:
return HttpResponseRedirect(reverse('zerver.views.registration.find_account'))
if realm.deactivated:
return redirect_to_deactivation_notice()

View File

@ -653,8 +653,9 @@ def social_associate_user_helper(backend: BaseAuth, return_data: Dict[str, Any],
Returns a UserProfile object for successful authentication, and None otherwise.
"""
subdomain = backend.strategy.session_get('subdomain')
realm = get_realm(subdomain)
if realm is None:
try:
realm = get_realm(subdomain)
except Realm.DoesNotExist:
return_data["invalid_realm"] = True
return None
return_data["realm_id"] = realm.id