remote_server: Migrate RemoteZulipServer.uuid to be UUIDField.

Given that these values are uuids, it's better to use UUIDField which is
meant for exactly that, rather than an arbitrary CharField.

This requires modifying some tests to use valid uuids.
This commit is contained in:
Mateusz Mandera 2021-12-22 14:37:12 +01:00 committed by Tim Abbott
parent e48120fd12
commit c5c3ab66d6
7 changed files with 43 additions and 17 deletions

View File

@ -4,6 +4,7 @@ import os
import random
import re
import sys
import uuid
from dataclasses import dataclass
from datetime import datetime, timedelta, timezone
from decimal import Decimal
@ -4424,7 +4425,7 @@ class BillingHelpersTest(ZulipTestCase):
self.assertTrue(is_sponsored_realm(realm))
def test_change_remote_server_plan_type(self) -> None:
server_uuid = "demo-1234"
server_uuid = str(uuid.uuid4())
remote_server = RemoteZulipServer.objects.create(
uuid=server_uuid,
api_key="magic_secret_api_key",

View File

@ -50,6 +50,7 @@ IGNORED_PHRASES = [
r"URL",
r"Ubuntu",
r"Updown",
r"UUID",
r"V5",
r"Webathena",
r"Windows",

View File

@ -1,6 +1,7 @@
import base64
import os
import re
import uuid
from collections import defaultdict
from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple
from unittest import mock, skipUnless
@ -644,7 +645,7 @@ class RateLimitTestCase(ZulipTestCase):
@skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
def test_rate_limiting_happens_if_remote_server(self) -> None:
server_uuid = "1234-abcd"
server_uuid = str(uuid.uuid4())
server = RemoteZulipServer(
uuid=server_uuid,
api_key="magic_secret_api_key",

View File

@ -1,4 +1,5 @@
import time
import uuid
from contextlib import contextmanager
from typing import IO, Any, Callable, Iterator, Optional, Sequence
from unittest import mock, skipUnless
@ -411,7 +412,7 @@ class RateLimitTests(ZulipTestCase):
@skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
@rate_limit_rule(1, 5, domain="api_by_remote_server")
def test_hit_ratelimits_as_remote_server(self) -> None:
server_uuid = "1234-abcd"
server_uuid = str(uuid.uuid4())
server = RemoteZulipServer(
uuid=server_uuid,
api_key="magic_secret_api_key",
@ -433,7 +434,7 @@ class RateLimitTests(ZulipTestCase):
self.assertEqual(
m.output,
[
"WARNING:zerver.lib.rate_limiter:Remote server <RemoteZulipServer demo.example.com 1234-abcd> exceeded rate limits on domain api_by_remote_server"
f"WARNING:zerver.lib.rate_limiter:Remote server <RemoteZulipServer demo.example.com {server_uuid[:12]}> exceeded rate limits on domain api_by_remote_server"
],
)
finally:

View File

@ -90,7 +90,7 @@ if settings.ZILENCER_ENABLED:
@skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
class BouncerTestCase(ZulipTestCase):
def setUp(self) -> None:
self.server_uuid = "1234-abcd"
self.server_uuid = "6cde5f7a-1f7e-4978-9716-49f69ebfc9fe"
server = RemoteZulipServer(
uuid=self.server_uuid,
api_key="magic_secret_api_key",
@ -234,12 +234,15 @@ class PushBouncerNotificationTest(BouncerTestCase):
self.server_uuid, endpoint, dict(user_id=user_id, token_kind=token_kind, token=token)
)
self.assert_json_error(
result, "Zulip server auth failure: key does not match role 1234-abcd", status_code=401
result,
"Zulip server auth failure: key does not match role 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe",
status_code=401,
)
del self.API_KEYS[self.server_uuid]
credentials = "{}:{}".format("5678-efgh", "invalid")
credentials_uuid = str(uuid.uuid4())
credentials = "{}:{}".format(credentials_uuid, "invalid")
api_auth = "Basic " + base64.b64encode(credentials.encode()).decode()
result = self.client_post(
endpoint,
@ -248,7 +251,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
)
self.assert_json_error(
result,
"Zulip server auth failure: 5678-efgh is not registered -- did you run `manage.py register_server`?",
f"Zulip server auth failure: {credentials_uuid} is not registered -- did you run `manage.py register_server`?",
status_code=401,
)
@ -325,7 +328,7 @@ class PushBouncerNotificationTest(BouncerTestCase):
logger.output,
[
"INFO:zilencer.views:"
f"Sending mobile push notifications for remote user 1234-abcd:{hamlet.id}: "
f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{hamlet.id}: "
"2 via FCM devices, 1 via APNs devices"
],
)
@ -673,7 +676,7 @@ class AnalyticsBouncerTest(BouncerTestCase):
self.assertEqual(
warn_log.output,
[
"WARNING:root:Invalid data saving zilencer_remoteinstallationcount for server demo.example.com/1234-abcd"
"WARNING:root:Invalid data saving zilencer_remoteinstallationcount for server demo.example.com/6cde5f7a-1f7e-4978-9716-49f69ebfc9fe"
],
)
@ -784,7 +787,7 @@ class AnalyticsBouncerTest(BouncerTestCase):
send_analytics_to_remote_server()
remote_log_entry = RemoteRealmAuditLog.objects.order_by("id").last()
assert remote_log_entry is not None
self.assertEqual(remote_log_entry.server.uuid, self.server_uuid)
self.assertEqual(str(remote_log_entry.server.uuid), self.server_uuid)
self.assertEqual(remote_log_entry.remote_id, log_entry.id)
self.assertEqual(remote_log_entry.event_time, self.TIME_ZERO)
self.assertEqual(remote_log_entry.backfilled, True)
@ -921,7 +924,7 @@ class HandlePushNotificationTest(PushNotificationTest):
views_logger.output,
[
"INFO:zilencer.views:"
f"Sending mobile push notifications for remote user 1234-abcd:{self.user_profile.id}: "
f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{self.user_profile.id}: "
f"{len(gcm_devices)} via FCM devices, {len(apns_devices)} via APNs devices"
],
)
@ -982,7 +985,7 @@ class HandlePushNotificationTest(PushNotificationTest):
views_logger.output,
[
"INFO:zilencer.views:"
f"Sending mobile push notifications for remote user 1234-abcd:{self.user_profile.id}: "
f"Sending mobile push notifications for remote user 6cde5f7a-1f7e-4978-9716-49f69ebfc9fe:{self.user_profile.id}: "
f"{len(gcm_devices)} via FCM devices, {len(apns_devices)} via APNs devices"
],
)

View File

@ -0,0 +1,18 @@
# Generated by Django 3.2.9 on 2021-12-22 10:06
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("zilencer", "0020_remotezulipserverauditlog"),
]
operations = [
migrations.AlterField(
model_name="remotezulipserver",
name="uuid",
field=models.UUIDField(unique=True),
),
]

View File

@ -1,5 +1,6 @@
import datetime
from typing import List, Tuple
from uuid import UUID
from django.conf import settings
from django.db import models
@ -26,7 +27,7 @@ class RemoteZulipServer(models.Model):
# The unique UUID (`zulip_org_id`) and API key (`zulip_org_key`)
# for this remote server registration.
uuid: str = models.CharField(max_length=UUID_LENGTH, unique=True)
uuid: UUID = models.UUIDField(unique=True)
api_key: str = models.CharField(max_length=API_KEY_LENGTH)
# The hostname and contact details are not verified/trusted. Thus,
@ -44,10 +45,10 @@ class RemoteZulipServer(models.Model):
plan_type: int = models.PositiveSmallIntegerField(default=PLAN_TYPE_SELF_HOSTED)
def __str__(self) -> str:
return f"<RemoteZulipServer {self.hostname} {self.uuid[0:12]}>"
return f"<RemoteZulipServer {self.hostname} {str(self.uuid)[0:12]}>"
def format_requestor_for_logs(self) -> str:
return "zulip-server:" + self.uuid
return "zulip-server:" + str(self.uuid)
class RemotePushDeviceToken(AbstractPushDeviceToken):
@ -137,7 +138,7 @@ class RateLimitedRemoteZulipServer(RateLimitedObject):
assert not settings.RUNNING_INSIDE_TORNADO
assert settings.ZILENCER_ENABLED
self.uuid = remote_server.uuid
self.uuid = str(remote_server.uuid)
self.domain = domain
super().__init__()