Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

pointer: Remove pointer from API and page_params. 

There is still some miscellaneous cleanup that
has to happen for things like analytics queries
and dead code in node tests, but this should
remove the main use of pointers in the backend.

(We will also still need to drop the DB field.)
This commit is contained in:
Steve Howell 2020-06-26 17:51:10 +00:00 committed by Tim Abbott
parent 256982b3f8
commit 69be97e365
15 changed files with 23 additions and 250 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
templates/zerver/api/changelog.md
View File

@ -10,6 +10,12 @@ below features are supported.
## Changes in Zulip 3.0
**Feature level 23**
* `GET/PUT/POST /users/me/pointer`: Eliminated. We eliminated
the whole concept of the "global" user pointer leading up to
this commit.
**Feature level 22**
* `GET /attachments`: The date when a message using the attachment was

2
version.py
View File

@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0"
#
# Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md.
API_FEATURE_LEVEL = 22
API_FEATURE_LEVEL = 23
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

36
zerver/lib/actions.py
View File

@ -4031,42 +4031,6 @@ def update_user_presence(user_profile: UserProfile, client: Client, log_time: da
if new_user_input:
update_user_activity_interval(user_profile, log_time)
def do_update_pointer(user_profile: UserProfile, client: Client,
pointer: int, update_flags: bool=False) -> None:
prev_pointer = user_profile.pointer
user_profile.pointer = pointer
user_profile.save(update_fields=["pointer"])
if update_flags: # nocoverage
# This block of code is compatibility code for the
# legacy/original Zulip Android app natively. It's a shim
# that will mark as read any messages up until the pointer
# move; we expect to remove this feature entirely before long,
# when we drop support for the old Android app entirely.
app_message_ids = UserMessage.objects.filter(
user_profile=user_profile,
message__id__gt=prev_pointer,
message__id__lte=pointer).extra(where=[
UserMessage.where_unread(),
UserMessage.where_active_push_notification(),
]).values_list("message_id", flat=True)
UserMessage.objects.filter(user_profile=user_profile,
message__id__gt=prev_pointer,
message__id__lte=pointer).extra(where=[UserMessage.where_unread()]) \
.update(flags=F('flags').bitor(UserMessage.flags.read))
do_clear_mobile_push_notifications_for_ids([user_profile.id], app_message_ids)
event_time = timezone_now()
count = len(app_message_ids)
do_increment_logging_stat(user_profile, COUNT_STATS['messages_read::hour'],
None, event_time, increment=count)
do_increment_logging_stat(user_profile, COUNT_STATS['messages_read_interactions::hour'],
None, event_time, increment=min(1, count))
event = dict(type='pointer', pointer=pointer)
send_event(user_profile.realm, event, [user_profile.id])
def do_update_user_status(user_profile: UserProfile,
away: Optional[bool],
status_text: Optional[str],

5
zerver/lib/events.py
View File

@ -131,9 +131,6 @@ def fetch_initial_state_data(user_profile: UserProfile,
if want('muted_topics'):
state['muted_topics'] = get_topic_mutes(user_profile)
if want('pointer'):
state['pointer'] = user_profile.pointer
if want('presence'):
state['presences'] = get_presences_for_realm(realm, slim_presence)
@ -398,8 +395,6 @@ def apply_event(state: Dict[str, Any],
state['hotspots'] = event['hotspots']
elif event['type'] == "custom_profile_fields":
state['custom_profile_fields'] = event['fields']
elif event['type'] == "pointer":
state['pointer'] = max(state['pointer'], event['pointer'])
elif event['type'] == "realm_user":
person = event['person']
person_user_id = person['user_id']

2
zerver/openapi/openapi.py
View File

@ -20,7 +20,7 @@ EXCLUDE_PROPERTIES = {
},
'/register': {
'post': {
'200': ['max_message_id', 'realm_emoji', 'pointer'],
'200': ['max_message_id', 'realm_emoji'],
},
},
'/settings/notifications': {

14
zerver/tests/test_events.py
View File

@ -86,7 +86,6 @@ from zerver.lib.actions import (
do_update_message,
do_update_message_flags,
do_update_outgoing_webhook_service,
do_update_pointer,
do_update_user_custom_profile_data_if_changed,
do_update_user_group_description,
do_update_user_group_name,
@ -293,9 +292,7 @@ class EventsEndpointTest(ZulipTestCase):
# Check that we didn't fetch the messages data
self.assertNotIn('max_message_id', result_dict)
# Check that the realm_emoji data is in there, and is correctly
# updated (presering our atomicity guaranteed), though of
# course any future pointer events won't be distributed
# Check that the realm_emoji data is in there.
self.assertIn('realm_emoji', result_dict)
self.assertEqual(result_dict['realm_emoji'], [])
self.assertEqual(result_dict['queue_id'], '15:13')
@ -1252,14 +1249,6 @@ class EventsRegisterTest(ZulipTestCase):
self.user_profile, get_client("ZulipAndroid/1.0"), timezone_now(), UserPresence.IDLE))
schema_checker_android('events[0]', events[0])
def test_pointer_events(self) -> None:
schema_checker = self.check_events_dict([
('type', equals('pointer')),
('pointer', check_int),
])
events = self.do_test(lambda: do_update_pointer(self.user_profile, get_client("website"), 1500))
schema_checker('events[0]', events[0])
def test_register_events(self) -> None:
realm_user_add_checker = self.check_events_dict([
('type', equals('realm_user')),
@ -3801,7 +3790,6 @@ class FetchQueriesTest(ZulipTestCase):
hotspots=0,
message=1,
muted_topics=1,
pointer=0,
presence=1,
realm=0,
realm_bot=1,

11
zerver/tests/test_home.py
View File

@ -122,7 +122,6 @@ class HomeTest(ZulipTestCase):
"password_min_guesses",
"password_min_length",
"pm_content_in_desktop_notifications",
"pointer",
"poll_timeout",
"presence_enabled",
"presences",
@ -456,15 +455,6 @@ class HomeTest(ZulipTestCase):
self.assertEqual(mock.call_args_list[0][0][0], "Invalid narrow requested, ignoring")
self._sanity_check(result)
def test_bad_pointer(self) -> None:
user_profile = self.example_user('hamlet')
user_profile.pointer = 999999
user_profile.save()
self.login_user(user_profile)
result = self._get_home_page()
self._sanity_check(result)
def test_topic_narrow(self) -> None:
self.login('hamlet')
result = self._get_home_page(stream='Denmark', topic='lunch')
@ -658,7 +648,6 @@ class HomeTest(ZulipTestCase):
page_params = self._get_page_params(result)
self.assertEqual(page_params['narrow_stream'], stream_name)
self.assertEqual(page_params['narrow'], [dict(operator='stream', operand=stream_name)])
self.assertEqual(page_params['pointer'], -1)
self.assertEqual(page_params['max_message_id'], -1)
def test_invites_by_admins_only(self) -> None:

2
zerver/tests/test_openapi.py
View File

@ -283,8 +283,6 @@ class OpenAPIArgumentsTest(ZulipTestCase):
'/settings/display',
# Much more valuable would be an org admin bulk-upload feature.
'/users/me/profile_data',
# To be deprecated and deleted.
'/users/me/pointer',
#### Should be documented as part of interactive bots documentation
'/bot_storage',

93
zerver/tests/test_unread.py
View File

@ -8,77 +8,19 @@ from zerver.lib.fix_unreads import fix, fix_unsubscribed
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import get_subscription, tornado_redirected_to_list
from zerver.lib.topic_mutes import add_topic_mute
from zerver.models import Subscription, UserMessage, UserProfile, get_realm, get_stream, get_user
from zerver.models import Subscription, UserMessage, UserProfile, get_realm, get_stream
class PointerTest(ZulipTestCase):
class FirstUnreadAnchorTests(ZulipTestCase):
'''
HISTORICAL NOTE:
def test_update_pointer(self) -> None:
"""
Posting a pointer to /update (in the form {"pointer": pointer}) changes
the pointer we store for your UserProfile.
"""
The two tests in this class were originally written when
we had the concept of a "pointer", and they may be a bit
redundant in what they now check.
'''
def test_use_first_unread_anchor(self) -> None:
self.login('hamlet')
self.assertEqual(self.example_user('hamlet').pointer, -1)
msg_id = self.send_stream_message(self.example_user("othello"), "Verona")
result = self.client_post("/json/users/me/pointer", {"pointer": msg_id})
self.assert_json_success(result)
self.assertEqual(self.example_user('hamlet').pointer, msg_id)
def test_api_update_pointer(self) -> None:
"""
Same as above, but for the API view
"""
user = self.example_user('hamlet')
email = user.email
self.assertEqual(user.pointer, -1)
msg_id = self.send_stream_message(self.example_user("othello"), "Verona")
result = self.api_post(user, "/api/v1/users/me/pointer", {"pointer": msg_id})
self.assert_json_success(result)
self.assertEqual(get_user(email, user.realm).pointer, msg_id)
def test_missing_pointer(self) -> None:
"""
Posting json to /json/users/me/pointer which does not contain a pointer key/value pair
returns a 400 and error message.
"""
self.login('hamlet')
self.assertEqual(self.example_user('hamlet').pointer, -1)
result = self.client_post("/json/users/me/pointer", {"foo": 1})
self.assert_json_error(result, "Missing 'pointer' argument")
self.assertEqual(self.example_user('hamlet').pointer, -1)
def test_invalid_pointer(self) -> None:
"""
Posting json to /json/users/me/pointer with an invalid pointer returns a 400 and error
message.
"""
self.login('hamlet')
self.assertEqual(self.example_user('hamlet').pointer, -1)
result = self.client_post("/json/users/me/pointer", {"pointer": "foo"})
self.assert_json_error(result, "Bad value for 'pointer': foo")
self.assertEqual(self.example_user('hamlet').pointer, -1)
def test_pointer_out_of_range(self) -> None:
"""
Posting json to /json/users/me/pointer with an out of range (< 0) pointer returns a 400
and error message.
"""
self.login('hamlet')
self.assertEqual(self.example_user('hamlet').pointer, -1)
result = self.client_post("/json/users/me/pointer", {"pointer": -2})
self.assert_json_error(result, "Bad value for 'pointer': -2")
self.assertEqual(self.example_user('hamlet').pointer, -1)
def test_use_first_unread_anchor_interaction_with_pointer(self) -> None:
"""
Getting old messages (a get request to /json/messages) should never
return an unread message older than the current pointer, when there's
no narrow set.
"""
self.login('hamlet')
# Ensure the pointer is not set (-1)
self.assertEqual(self.example_user('hamlet').pointer, -1)
# Mark all existing messages as read
result = self.client_post("/json/mark_all_as_read")
@ -107,7 +49,6 @@ class PointerTest(ZulipTestCase):
messages = self.get_messages(
anchor=0, num_before=0, num_after=2, use_first_unread_anchor=False)
old_message_id = messages[0]['id']
next_old_message_id = messages[1]['id']
# Verify the message is marked as read
user_message = UserMessage.objects.get(
@ -135,23 +76,8 @@ class PointerTest(ZulipTestCase):
self.assertEqual(messages_response['messages'][0]['id'], old_message_id)
self.assertEqual(messages_response['anchor'], old_message_id)
# Let's update the pointer to be *after* this old unread message (but
# still on or before the new unread message we just sent)
result = self.client_post("/json/users/me/pointer",
{"pointer": next_old_message_id})
self.assert_json_success(result)
self.assertEqual(self.example_user('hamlet').pointer,
next_old_message_id)
# Verify that moving the pointer didn't mark our message as read.
user_message = UserMessage.objects.get(
message_id=old_message_id,
user_profile=self.example_user('hamlet'))
self.assertFalse(user_message.flags.read)
def test_visible_messages_use_first_unread_anchor(self) -> None:
self.login('hamlet')
self.assertEqual(self.example_user('hamlet').pointer, -1)
result = self.client_post("/json/mark_all_as_read")
self.assert_json_success(result)
@ -184,6 +110,7 @@ class PointerTest(ZulipTestCase):
anchor="first_unread", num_before=0, num_after=1)
self.assert_length(messages, 1)
class UnreadCountTests(ZulipTestCase):
def setUp(self) -> None:
super().setUp()

6
zerver/tests/test_urls.py
View File

@ -64,7 +64,6 @@ class PublicURLTest(ZulipTestCase):
"/json/subscriptions/exists",
"/api/v1/users/me/subscriptions/properties",
"/json/fetch_api_key",
"/json/users/me/pointer",
"/json/users/me/subscriptions",
"/api/v1/users/me/subscriptions",
"/json/export/realm",
@ -76,16 +75,13 @@ class PublicURLTest(ZulipTestCase):
patch_urls = {
401: ["/json/settings"],
}
put_urls = {401: ["/json/users/me/pointer"],
}
for status_code, url_set in get_urls.items():
self.fetch("client_get", url_set, status_code)
for status_code, url_set in post_urls.items():
self.fetch("client_post", url_set, status_code)
for status_code, url_set in patch_urls.items():
self.fetch("client_patch", url_set, status_code)
for status_code, url_set in put_urls.items():
self.fetch("client_put", url_set, status_code)
def test_get_gcid_when_not_configured(self) -> None:
with self.settings(GOOGLE_CLIENT_ID=None):

46
zerver/tests/test_users.py
View File

@ -1531,25 +1531,6 @@ class BulkUsersTest(ZulipTestCase):
class GetProfileTest(ZulipTestCase):
def common_update_pointer(self, user: UserProfile, pointer: int) -> None:
self.login_user(user)
result = self.client_post("/json/users/me/pointer", {"pointer": pointer})
self.assert_json_success(result)
def common_get_pointer(self, user_id: str) -> Dict[str, Any]:
user_profile = self.example_user(user_id)
result = self.api_get(user_profile, "/json/users/me/pointer")
self.assert_json_success(result)
json = result.json()
return json
def test_get_pointer(self) -> None:
user = self.example_user("hamlet")
self.login_user(user)
result = self.client_get("/json/users/me/pointer")
self.assert_json_success(result)
self.assertIn("pointer", result.json())
def test_cache_behavior(self) -> None:
"""Tests whether fetching a user object the normal way, with
`get_user`, makes 1 cache query and 1 database query.
@ -1617,33 +1598,6 @@ class GetProfileTest(ZulipTestCase):
self.assertEqual(result['user']['email'], bot.email)
self.assertTrue(result['user']['is_bot'])
def test_api_get_empty_profile(self) -> None:
"""
Ensure GET /users/me returns a max message id and returns successfully
"""
json = self.common_get_pointer("othello")
self.assertEqual(json['pointer'], -1)
def test_profile_with_pointer(self) -> None:
"""
Ensure GET /users/me returns a proper pointer id after the pointer is updated
"""
id1 = self.send_stream_message(self.example_user("othello"), "Verona")
id2 = self.send_stream_message(self.example_user("othello"), "Verona")
hamlet = self.example_user('hamlet')
self.common_update_pointer(hamlet, id2)
json = self.common_get_pointer("hamlet")
self.assertEqual(json["pointer"], id2)
self.common_update_pointer(hamlet, id1)
json = self.common_get_pointer("hamlet")
self.assertEqual(json["pointer"], id2) # pointer does not move backwards
result = self.client_post("/json/users/me/pointer", {"pointer": 99999999})
self.assert_json_error(result, "Invalid message ID")
def test_get_all_profiles_avatar_urls(self) -> None:
hamlet = self.example_user('hamlet')
result = self.api_get(hamlet, "/api/v1/users")

7
zerver/tornado/event_queue.py
View File

@ -303,7 +303,7 @@ class EventQueue:
event['id'] = self.next_event_id
self.next_event_id += 1
full_event_type = compute_full_event_type(event)
if (full_event_type in ["pointer", "restart"] or
if (full_event_type == "restart" or
full_event_type.startswith("flags/")):
if full_event_type not in self.virtual_events:
self.virtual_events[full_event_type] = copy.deepcopy(event)
@ -313,9 +313,8 @@ class EventQueue:
virtual_event["id"] = event["id"]
if "timestamp" in event:
virtual_event["timestamp"] = event["timestamp"]
if full_event_type == "pointer":
virtual_event["pointer"] = event["pointer"]
elif full_event_type == "restart":
if full_event_type == "restart":
virtual_event["server_generation"] = event["server_generation"]
elif full_event_type.startswith("flags/"):
virtual_event["messages"] += event["messages"]

10
zerver/views/home.py
View File

@ -212,15 +212,6 @@ def home_real(request: HttpRequest) -> HttpResponse:
)
needs_tutorial = user_profile.tutorial_status == UserProfile.TUTORIAL_WAITING
if user_profile.pointer == -1:
# Put the new user's pointer at the bottom
#
# This improves performance, because we limit backfilling of messages
# before the pointer. It's also likely that someone joining an
# organization is interested in recent messages more than the very
# first messages on the system.
register_ret['pointer'] = register_ret['max_message_id']
else: # nocoverage
first_in_realm = False
prompt_for_invites = False
@ -291,7 +282,6 @@ def home_real(request: HttpRequest) -> HttpResponse:
page_params["narrow_topic"] = narrow_topic
page_params["narrow"] = [dict(operator=term[0], operand=term[1]) for term in narrow]
page_params["max_message_id"] = initial_pointer
page_params["pointer"] = initial_pointer
page_params["enable_desktop_notifications"] = False
statsd.incr('views.home')

27
zerver/views/pointer.py
View File

@ -1,27 +0,0 @@
from django.http import HttpRequest, HttpResponse
from django.utils.translation import ugettext as _
from zerver.lib.actions import do_update_pointer
from zerver.lib.request import REQ, JsonableError, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.validator import to_non_negative_int
from zerver.models import UserProfile, get_usermessage_by_message_id
def get_pointer_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:
return json_success({'pointer': user_profile.pointer})
@has_request_variables
def update_pointer_backend(request: HttpRequest, user_profile: UserProfile,
pointer: int=REQ(converter=to_non_negative_int)) -> HttpResponse:
if pointer <= user_profile.pointer:
return json_success()
if get_usermessage_by_message_id(user_profile, pointer) is None:
raise JsonableError(_("Invalid message ID"))
request._log_data["extra"] = f"[{pointer}]"
update_flags = (request.client.name.lower() in ['android', "zulipandroid"])
do_update_pointer(user_profile, request.client, pointer, update_flags=update_flags)
return json_success()

6
zproject/urls.py
View File

@ -260,12 +260,6 @@ v1_api_and_json_patterns = [
path('users/me', rest_dispatch,
{'GET': 'zerver.views.users.get_profile_backend',
'DELETE': 'zerver.views.users.deactivate_user_own_backend'}),
# PUT is currently used by mobile apps, we intend to remove the PUT version
# as soon as possible. POST exists to correct the erroneous use of PUT.
path('users/me/pointer', rest_dispatch,
{'GET': 'zerver.views.pointer.get_pointer_backend',
'PUT': 'zerver.views.pointer.update_pointer_backend',
'POST': 'zerver.views.pointer.update_pointer_backend'}),
path('users/me/presence', rest_dispatch,
{'POST': 'zerver.views.presence.update_active_status_backend'}),
path('users/me/status', rest_dispatch,