python: Remove unittest.mock.Mock uses from production code.

It’s somewhat expensive to import and confuses mypy.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2020-08-27 13:19:00 -07:00 committed by Tim Abbott
parent b31ff487c9
commit 51f993e084
3 changed files with 25 additions and 25 deletions

View File

@ -83,11 +83,6 @@ if settings.BILLING_ENABLED:
if settings.ZILENCER_ENABLED: if settings.ZILENCER_ENABLED:
from zilencer.models import RemoteInstallationCount, RemoteRealmCount, RemoteZulipServer from zilencer.models import RemoteInstallationCount, RemoteRealmCount, RemoteZulipServer
else:
from unittest.mock import Mock
RemoteInstallationCount = Mock() # type: ignore[misc] # https://github.com/JukkaL/mypy/issues/1188
RemoteZulipServer = Mock() # type: ignore[misc] # https://github.com/JukkaL/mypy/issues/1188
RemoteRealmCount = Mock() # type: ignore[misc] # https://github.com/JukkaL/mypy/issues/1188
MAX_TIME_FOR_FULL_ANALYTICS_GENERATION = timedelta(days=1, minutes=30) MAX_TIME_FOR_FULL_ANALYTICS_GENERATION = timedelta(days=1, minutes=30)
@ -134,6 +129,7 @@ def stats_for_realm(request: HttpRequest, realm_str: str) -> HttpResponse:
@has_request_variables @has_request_variables
def stats_for_remote_realm(request: HttpRequest, remote_server_id: int, def stats_for_remote_realm(request: HttpRequest, remote_server_id: int,
remote_realm_id: int) -> HttpResponse: remote_realm_id: int) -> HttpResponse:
assert settings.ZILENCER_ENABLED
server = RemoteZulipServer.objects.get(id=remote_server_id) server = RemoteZulipServer.objects.get(id=remote_server_id)
return render_stats(request, f'/remote/{server.id}/realm/{remote_realm_id}', return render_stats(request, f'/remote/{server.id}/realm/{remote_realm_id}',
f"Realm {remote_realm_id} on server {server.hostname}") f"Realm {remote_realm_id} on server {server.hostname}")
@ -154,6 +150,7 @@ def get_chart_data_for_realm(request: HttpRequest, user_profile: UserProfile,
def get_chart_data_for_remote_realm( def get_chart_data_for_remote_realm(
request: HttpRequest, user_profile: UserProfile, remote_server_id: int, request: HttpRequest, user_profile: UserProfile, remote_server_id: int,
remote_realm_id: int, **kwargs: Any) -> HttpResponse: remote_realm_id: int, **kwargs: Any) -> HttpResponse:
assert settings.ZILENCER_ENABLED
server = RemoteZulipServer.objects.get(id=remote_server_id) server = RemoteZulipServer.objects.get(id=remote_server_id)
return get_chart_data(request=request, user_profile=user_profile, server=server, return get_chart_data(request=request, user_profile=user_profile, server=server,
remote=True, remote_realm_id=int(remote_realm_id), **kwargs) remote=True, remote_realm_id=int(remote_realm_id), **kwargs)
@ -164,6 +161,7 @@ def stats_for_installation(request: HttpRequest) -> HttpResponse:
@require_server_admin @require_server_admin
def stats_for_remote_installation(request: HttpRequest, remote_server_id: int) -> HttpResponse: def stats_for_remote_installation(request: HttpRequest, remote_server_id: int) -> HttpResponse:
assert settings.ZILENCER_ENABLED
server = RemoteZulipServer.objects.get(id=remote_server_id) server = RemoteZulipServer.objects.get(id=remote_server_id)
return render_stats(request, f'/remote/{server.id}/installation', return render_stats(request, f'/remote/{server.id}/installation',
f'remote Installation {server.hostname}', True, True) f'remote Installation {server.hostname}', True, True)
@ -182,6 +180,7 @@ def get_chart_data_for_remote_installation(
remote_server_id: int, remote_server_id: int,
chart_name: str=REQ(), chart_name: str=REQ(),
**kwargs: Any) -> HttpResponse: **kwargs: Any) -> HttpResponse:
assert settings.ZILENCER_ENABLED
server = RemoteZulipServer.objects.get(id=remote_server_id) server = RemoteZulipServer.objects.get(id=remote_server_id)
return get_chart_data(request=request, user_profile=user_profile, for_installation=True, return get_chart_data(request=request, user_profile=user_profile, for_installation=True,
remote=True, server=server, **kwargs) remote=True, server=server, **kwargs)
@ -194,15 +193,17 @@ def get_chart_data(request: HttpRequest, user_profile: UserProfile, chart_name:
end: Optional[datetime]=REQ(converter=to_utc_datetime, default=None), end: Optional[datetime]=REQ(converter=to_utc_datetime, default=None),
realm: Optional[Realm]=None, for_installation: bool=False, realm: Optional[Realm]=None, for_installation: bool=False,
remote: bool=False, remote_realm_id: Optional[int]=None, remote: bool=False, remote_realm_id: Optional[int]=None,
server: Optional[RemoteZulipServer]=None) -> HttpResponse: server: Optional["RemoteZulipServer"]=None) -> HttpResponse:
if for_installation: if for_installation:
if remote: if remote:
assert settings.ZILENCER_ENABLED
aggregate_table = RemoteInstallationCount aggregate_table = RemoteInstallationCount
assert server is not None assert server is not None
else: else:
aggregate_table = InstallationCount aggregate_table = InstallationCount
else: else:
if remote: if remote:
assert settings.ZILENCER_ENABLED
aggregate_table = RemoteRealmCount aggregate_table = RemoteRealmCount
assert server is not None assert server is not None
assert remote_realm_id is not None assert remote_realm_id is not None
@ -309,20 +310,26 @@ def get_chart_data(request: HttpRequest, user_profile: UserProfile, chart_name:
aggregation_level = { aggregation_level = {
InstallationCount: 'everyone', InstallationCount: 'everyone',
RealmCount: 'everyone', RealmCount: 'everyone',
RemoteInstallationCount: 'everyone',
RemoteRealmCount: 'everyone',
UserCount: 'user', UserCount: 'user',
} }
if settings.ZILENCER_ENABLED:
aggregation_level[RemoteInstallationCount] = 'everyone'
aggregation_level[RemoteRealmCount] = 'everyone'
# -1 is a placeholder value, since there is no relevant filtering on InstallationCount # -1 is a placeholder value, since there is no relevant filtering on InstallationCount
id_value = { id_value = {
InstallationCount: -1, InstallationCount: -1,
RealmCount: realm.id, RealmCount: realm.id,
RemoteInstallationCount: server.id if server is not None else None,
# TODO: RemoteRealmCount logic doesn't correctly handle
# filtering by server_id as well.
RemoteRealmCount: remote_realm_id,
UserCount: user_profile.id, UserCount: user_profile.id,
} }
if settings.ZILENCER_ENABLED:
if server is not None:
id_value[RemoteInstallationCount] = server.id
# TODO: RemoteRealmCount logic doesn't correctly handle
# filtering by server_id as well.
if remote_realm_id is not None:
id_value[RemoteRealmCount] = remote_realm_id
for table in tables: for table in tables:
data[aggregation_level[table]] = {} data[aggregation_level[table]] = {}
for stat in stats: for stat in stats:
@ -366,9 +373,9 @@ def table_filtered_to_id(table: Type[BaseCount], key_id: int) -> QuerySet:
return StreamCount.objects.filter(stream_id=key_id) return StreamCount.objects.filter(stream_id=key_id)
elif table == InstallationCount: elif table == InstallationCount:
return InstallationCount.objects.all() return InstallationCount.objects.all()
elif table == RemoteInstallationCount: elif settings.ZILENCER_ENABLED and table == RemoteInstallationCount:
return RemoteInstallationCount.objects.filter(server_id=key_id) return RemoteInstallationCount.objects.filter(server_id=key_id)
elif table == RemoteRealmCount: elif settings.ZILENCER_ENABLED and table == RemoteRealmCount:
return RemoteRealmCount.objects.filter(realm_id=key_id) return RemoteRealmCount.objects.filter(realm_id=key_id)
else: else:
raise AssertionError(f"Unknown table: {table}") raise AssertionError(f"Unknown table: {table}")

View File

@ -52,14 +52,8 @@ from zerver.lib.user_agent import parse_user_agent
from zerver.lib.utils import has_api_key_format, statsd from zerver.lib.utils import has_api_key_format, statsd
from zerver.models import Realm, UserProfile, get_client, get_user_profile_by_api_key from zerver.models import Realm, UserProfile, get_client, get_user_profile_by_api_key
# This is a hack to ensure that RemoteZulipServer always exists even
# if Zilencer isn't enabled.
if settings.ZILENCER_ENABLED: if settings.ZILENCER_ENABLED:
from zilencer.models import RemoteZulipServer, get_remote_server_by_uuid from zilencer.models import RemoteZulipServer, get_remote_server_by_uuid
else: # nocoverage # Hack here basically to make impossible code paths compile
from unittest.mock import Mock
get_remote_server_by_uuid = Mock()
RemoteZulipServer = Mock() # type: ignore[misc] # https://github.com/JukkaL/mypy/issues/1188
webhook_logger = logging.getLogger("zulip.zerver.webhooks") webhook_logger = logging.getLogger("zulip.zerver.webhooks")
log_to_file(webhook_logger, settings.API_KEY_ONLY_WEBHOOK_LOG_PATH) log_to_file(webhook_logger, settings.API_KEY_ONLY_WEBHOOK_LOG_PATH)
@ -204,7 +198,7 @@ class InvalidZulipServerKeyError(InvalidZulipServerError):
def validate_api_key(request: HttpRequest, role: Optional[str], def validate_api_key(request: HttpRequest, role: Optional[str],
api_key: str, is_webhook: bool=False, api_key: str, is_webhook: bool=False,
client_name: Optional[str]=None) -> Union[UserProfile, RemoteZulipServer]: client_name: Optional[str]=None) -> Union[UserProfile, "RemoteZulipServer"]:
# Remove whitespace to protect users from trivial errors. # Remove whitespace to protect users from trivial errors.
api_key = api_key.strip() api_key = api_key.strip()
if role is not None: if role is not None:

View File

@ -40,11 +40,8 @@ logger = logging.getLogger(__name__)
if settings.ZILENCER_ENABLED: if settings.ZILENCER_ENABLED:
from zilencer.models import RemotePushDeviceToken from zilencer.models import RemotePushDeviceToken
else: # nocoverage -- Not convenient to add test for this.
from unittest.mock import Mock
RemotePushDeviceToken = Mock() # type: ignore[misc] # https://github.com/JukkaL/mypy/issues/1188
DeviceToken = Union[PushDeviceToken, RemotePushDeviceToken] DeviceToken = Union[PushDeviceToken, "RemotePushDeviceToken"]
# We store the token as b64, but apns-client wants hex strings # We store the token as b64, but apns-client wants hex strings
def b64_to_hex(data: str) -> str: def b64_to_hex(data: str) -> str:
@ -123,6 +120,7 @@ def send_apple_push_notification(user_id: int, devices: List[DeviceToken],
return return
if remote: if remote:
assert settings.ZILENCER_ENABLED
DeviceTokenClass = RemotePushDeviceToken DeviceTokenClass = RemotePushDeviceToken
else: else:
DeviceTokenClass = PushDeviceToken DeviceTokenClass = PushDeviceToken
@ -291,6 +289,7 @@ def send_android_push_notification(devices: List[DeviceToken], data: Dict[str, A
logger.info("GCM: Sent %s as %s", reg_id, msg_id) logger.info("GCM: Sent %s as %s", reg_id, msg_id)
if remote: if remote:
assert settings.ZILENCER_ENABLED
DeviceTokenClass = RemotePushDeviceToken DeviceTokenClass = RemotePushDeviceToken
else: else:
DeviceTokenClass = PushDeviceToken DeviceTokenClass = PushDeviceToken