paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-10 00:46:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

Introduce client_message_id on the server.

Browse Source 
We are deprecating local_id/local_message_id on the Python server.
Instead of the server knowing about the client's implementation of
local id, with the message id = 9999.01 scheme, we just send the
server an opaque id to send back to us.

This commit changes the name from local_id -> client_message_id,
but it doesn't change the actual values passed yet.

The goal for client_key in future commits will be to:
    * Have it for all messages, not just locally rendered messages
    * Not have it overlap with server-side message ids.

The history behind local_id having numbers like 9999.01 is that
they are actually interim message ids and the numerical value is
used for rendering the message list when we do client-side rendering.
This commit is contained in:
Steve Howell
2017-07-12 18:16:22 -04:00
committed by showell
parent bab96ab8a0
commit 8fbb55df85
10 changed files with 40 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
frontend_tests/node_tests/compose.js
View File

@@ -379,10 +379,11 @@ people.add(bob);
reply_to: 'alice@example.com', reply_to: 'alice@example.com',
private_message_recipient: 'alice@example.com', private_message_recipient: 'alice@example.com',
to_user_ids: '31', to_user_ids: '31',
client_message_id: 1,
local_id: 1, local_id: 1,
}; };
assert.equal(payload.url, '/json/messages'); assert.equal(payload.url, '/json/messages');
assert.equal(_.keys(payload.data).length, 11); assert.equal(_.keys(payload.data).length, 12);
assert.deepEqual(payload.data, single_msg); assert.deepEqual(payload.data, single_msg);
payload.data.id = stub_state.local_id_counter; payload.data.id = stub_state.local_id_counter;
payload.success(payload.data); payload.success(payload.data);
@@ -803,6 +804,7 @@ function test_with_mock_socket(test_params) {
assert.equal(send_args.request, request); assert.equal(send_args.request, request);
assert.deepEqual(send_args.request, { assert.deepEqual(send_args.request, {
foo: 'bar', foo: 'bar',
client_message_id: undefined,
socket_user_agent: 'unittest_transmit_message', socket_user_agent: 'unittest_transmit_message',
}); });

7
static/js/compose.js
View File

@@ -302,6 +302,13 @@ exports.send_message_success = function (local_id, message_id, start_time, local
}; };
exports.transmit_message = function (request, success, error) { exports.transmit_message = function (request, success, error) {
// Give the server our local_id as client_message_id. We
// will eventually decouple these concepts, so that local_id
// is only an interim value for a message id, whereas
// client_message_id will be a key used to track the message through
// its lifecycle of being posted/received/acked/sent/displayed.
request.client_message_id = request.local_id;
delete exports.send_times_data[request.id]; delete exports.send_times_data[request.id];
if (page_params.use_websockets) { if (page_params.use_websockets) {
send_message_socket(request, success, error); send_message_socket(request, success, error);

7
static/js/server_events.js
View File

@@ -68,8 +68,11 @@ function get_events_success(events) {
case 'message': case 'message':
var msg = event.message; var msg = event.message;
msg.flags = event.flags; msg.flags = event.flags;
if (event.local_message_id !== undefined) {
msg.local_id = event.local_message_id; // We will eventually decouple msg.local_id from event.client_message_id,
// but for now they are the same value.
if (event.client_message_id !== undefined) {
msg.local_id = event.client_message_id;
} }
messages.push(msg); messages.push(msg);
break; break;

14
zerver/lib/actions.py
View File

@@ -753,7 +753,7 @@ def do_send_messages(messages_maybe_none):
for message in messages: for message in messages:
message['rendered_content'] = message.get('rendered_content', None) message['rendered_content'] = message.get('rendered_content', None)
message['stream'] = message.get('stream', None) message['stream'] = message.get('stream', None)
message['local_id'] = message.get('local_id', None) message['client_message_id'] = message.get('client_message_id', None)
message['sender_queue_id'] = message.get('sender_queue_id', None) message['sender_queue_id'] = message.get('sender_queue_id', None)
message['realm'] = message.get('realm', message['message'].sender.realm) message['realm'] = message.get('realm', message['message'].sender.realm)
@@ -899,8 +899,8 @@ def do_send_messages(messages_maybe_none):
event['stream_name'] = message['stream'].name event['stream_name'] = message['stream'].name
if message['stream'].invite_only: if message['stream'].invite_only:
event['invite_only'] = True event['invite_only'] = True
if message['local_id'] is not None: if message['client_message_id'] is not None:
event['local_id'] = message['local_id'] event['client_message_id'] = message['client_message_id']
if message['sender_queue_id'] is not None: if message['sender_queue_id'] is not None:
event['sender_queue_id'] = message['sender_queue_id'] event['sender_queue_id'] = message['sender_queue_id']
send_event(event, users) send_event(event, users)
@@ -1182,12 +1182,12 @@ def extract_recipients(s):
# Returns the id of the sent message. Has same argspec as check_message. # Returns the id of the sent message. Has same argspec as check_message.
def check_send_message(sender, client, message_type_name, message_to, def check_send_message(sender, client, message_type_name, message_to,
subject_name, message_content, realm=None, forged=False, subject_name, message_content, realm=None, forged=False,
forged_timestamp=None, forwarder_user_profile=None, local_id=None, forged_timestamp=None, forwarder_user_profile=None, client_message_id=None,
sender_queue_id=None): sender_queue_id=None):
# type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> int # type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> int
message = check_message(sender, client, message_type_name, message_to, message = check_message(sender, client, message_type_name, message_to,
subject_name, message_content, realm, forged, forged_timestamp, subject_name, message_content, realm, forged, forged_timestamp,
forwarder_user_profile, local_id, sender_queue_id) forwarder_user_profile, client_message_id, sender_queue_id)
return do_send_messages([message])[0] return do_send_messages([message])[0]
def check_stream_name(stream_name): def check_stream_name(stream_name):
@@ -1252,7 +1252,7 @@ def send_pm_if_empty_stream(sender, stream, stream_name, realm):
# Returns message ready for sending with do_send_message on success or the error message (string) on error. # Returns message ready for sending with do_send_message on success or the error message (string) on error.
def check_message(sender, client, message_type_name, message_to, def check_message(sender, client, message_type_name, message_to,
subject_name, message_content_raw, realm=None, forged=False, subject_name, message_content_raw, realm=None, forged=False,
forged_timestamp=None, forwarder_user_profile=None, local_id=None, forged_timestamp=None, forwarder_user_profile=None, client_message_id=None,
sender_queue_id=None): sender_queue_id=None):
# type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> Dict[str, Any] # type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> Dict[str, Any]
stream = None stream = None
@@ -1347,7 +1347,7 @@ def check_message(sender, client, message_type_name, message_to,
if id is not None: if id is not None:
return {'message': id} return {'message': id}
return {'message': message, 'stream': stream, 'local_id': local_id, return {'message': message, 'stream': stream, 'client_message_id': client_message_id,
'sender_queue_id': sender_queue_id, 'realm': realm} 'sender_queue_id': sender_queue_id, 'realm': realm}
def _internal_prep_message(realm, sender, recipient_type_name, parsed_recipients, def _internal_prep_message(realm, sender, recipient_type_name, parsed_recipients,

17
zerver/tests/test_events.py
View File

@@ -272,8 +272,9 @@ class GetEventsTest(ZulipTestCase):
self.assert_json_success(result) self.assert_json_success(result)
self.assert_length(events, 0) self.assert_length(events, 0)
local_id = 10.01 client_message_id = 'opaque-id-1'
self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", local_id=local_id, sender_queue_id=queue_id) self.send_message(email, recipient_email, Recipient.PERSONAL,
"hello", client_message_id=client_message_id, sender_queue_id=queue_id)
result = self.tornado_call(get_events_backend, user_profile, result = self.tornado_call(get_events_backend, user_profile,
{"queue_id": queue_id, {"queue_id": queue_id,
@@ -286,14 +287,14 @@ class GetEventsTest(ZulipTestCase):
self.assert_length(events, 1) self.assert_length(events, 1)
self.assertEqual(events[0]["type"], "message") self.assertEqual(events[0]["type"], "message")
self.assertEqual(events[0]["message"]["sender_email"], email) self.assertEqual(events[0]["message"]["sender_email"], email)
self.assertEqual(events[0]["local_message_id"], local_id) self.assertEqual(events[0]["client_message_id"], client_message_id)
self.assertEqual(events[0]["message"]["display_recipient"][0]["is_mirror_dummy"], False) self.assertEqual(events[0]["message"]["display_recipient"][0]["is_mirror_dummy"], False)
self.assertEqual(events[0]["message"]["display_recipient"][1]["is_mirror_dummy"], False) self.assertEqual(events[0]["message"]["display_recipient"][1]["is_mirror_dummy"], False)
last_event_id = events[0]["id"] last_event_id = events[0]["id"]
local_id += 0.01 client_message_id = 'opaque-id-2'
self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", local_id=local_id, sender_queue_id=queue_id) self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", client_message_id=client_message_id, sender_queue_id=queue_id)
result = self.tornado_call(get_events_backend, user_profile, result = self.tornado_call(get_events_backend, user_profile,
{"queue_id": queue_id, {"queue_id": queue_id,
@@ -306,7 +307,7 @@ class GetEventsTest(ZulipTestCase):
self.assert_length(events, 1) self.assert_length(events, 1)
self.assertEqual(events[0]["type"], "message") self.assertEqual(events[0]["type"], "message")
self.assertEqual(events[0]["message"]["sender_email"], email) self.assertEqual(events[0]["message"]["sender_email"], email)
self.assertEqual(events[0]["local_message_id"], local_id) self.assertEqual(events[0]["client_message_id"], client_message_id)
# Test that the received message in the receiver's event queue # Test that the received message in the receiver's event queue
# exists and does not contain a local id # exists and does not contain a local id
@@ -321,10 +322,10 @@ class GetEventsTest(ZulipTestCase):
self.assertEqual(len(recipient_events), 2) self.assertEqual(len(recipient_events), 2)
self.assertEqual(recipient_events[0]["type"], "message") self.assertEqual(recipient_events[0]["type"], "message")
self.assertEqual(recipient_events[0]["message"]["sender_email"], email) self.assertEqual(recipient_events[0]["message"]["sender_email"], email)
self.assertTrue("local_message_id" not in recipient_events[0]) self.assertTrue("client_message_id" not in recipient_events[0])
self.assertEqual(recipient_events[1]["type"], "message") self.assertEqual(recipient_events[1]["type"], "message")
self.assertEqual(recipient_events[1]["message"]["sender_email"], email) self.assertEqual(recipient_events[1]["message"]["sender_email"], email)
self.assertTrue("local_message_id" not in recipient_events[1]) self.assertTrue("client_message_id" not in recipient_events[1])
def test_get_events_narrow(self): def test_get_events_narrow(self):
# type: () -> None # type: () -> None

4
zerver/tests/test_tornado.py
View File

@@ -356,7 +356,7 @@ class TornadoTestCase(WebSocketBaseTestCase):
"queue_id": queue_events_data['response']['queue_id'], "queue_id": queue_events_data['response']['queue_id'],
"to": ujson.dumps([self.example_email('othello')]), "to": ujson.dumps([self.example_email('othello')]),
"reply_to": self.example_email('hamlet'), "reply_to": self.example_email('hamlet'),
"local_id": -1 "client_message_id": 'id-42',
} }
} }
user_message_str = ujson.dumps(user_message) user_message_str = ujson.dumps(user_message)
@@ -391,7 +391,7 @@ class TornadoTestCase(WebSocketBaseTestCase):
"queue_id": queue_events_data['response']['queue_id'], "queue_id": queue_events_data['response']['queue_id'],
"to": ujson.dumps(["Denmark"]), "to": ujson.dumps(["Denmark"]),
"reply_to": self.example_email('hamlet'), "reply_to": self.example_email('hamlet'),
"local_id": -1 "client_message_id": 'id-99',
} }
} }
user_message_str = ujson.dumps(user_message) user_message_str = ujson.dumps(user_message)

6
zerver/tornado/event_queue.py
View File

@@ -762,9 +762,9 @@ def process_message_event(event_template, users):
user_event.update(extra_data) user_event.update(extra_data)
if is_sender: if is_sender:
local_message_id = event_template.get('local_id', None) client_message_id = event_template.get('client_message_id', None)
if local_message_id is not None: if client_message_id is not None:
user_event["local_message_id"] = local_message_id user_event["client_message_id"] = client_message_id
if not client.accepts_event(user_event): if not client.accepts_event(user_event):
continue continue

2
zerver/tornado/socket.py
View File

@@ -262,7 +262,7 @@ def fake_message_sender(event):
msg_id = check_send_message(sender, client, req['type'], msg_id = check_send_message(sender, client, req['type'],
extract_recipients(req['to']), extract_recipients(req['to']),
req['subject'], req['content'], req['subject'], req['content'],
local_id=req.get('local_id', None), client_message_id=req.get('client_message_id', None),
sender_queue_id=req.get('queue_id', None)) sender_queue_id=req.get('queue_id', None))
resp = {"result": "success", "msg": "", "id": msg_id} resp = {"result": "success", "msg": "", "id": msg_id}
except JsonableError as e: except JsonableError as e:

2
zerver/tornado/websocket_client.py
View File

@@ -118,7 +118,7 @@ class WebsocketClient(object):
"queue_id": self.events_data['queue_id'], "queue_id": self.events_data['queue_id'],
"to": ujson.dumps([private_message_recepient]), "to": ujson.dumps([private_message_recepient]),
"reply_to": self.user_profile.email, "reply_to": self.user_profile.email,
"local_id": -1 "client_message_id": -1,
} }
} }
self.ws.write_message(ujson.dumps([ujson.dumps(user_message)])) self.ws.write_message(ujson.dumps([ujson.dumps(user_message)]))

4
zerver/views/messages.py
View File

@@ -923,7 +923,7 @@ def send_message_backend(request, user_profile,
subject_name = REQ('subject', lambda x: x.strip(), None), subject_name = REQ('subject', lambda x: x.strip(), None),
message_content = REQ('content'), message_content = REQ('content'),
realm_str = REQ('realm_str', default=None), realm_str = REQ('realm_str', default=None),
local_id = REQ(default=None), client_message_id = REQ(default=None),
queue_id = REQ(default=None)): queue_id = REQ(default=None)):
# type: (HttpRequest, UserProfile, Text, List[Text], bool, Optional[Text], Text, Optional[Text], Optional[Text], Optional[Text]) -> HttpResponse # type: (HttpRequest, UserProfile, Text, List[Text], bool, Optional[Text], Text, Optional[Text], Optional[Text], Optional[Text]) -> HttpResponse
client = request.client client = request.client
@@ -979,7 +979,7 @@ def send_message_backend(request, user_profile,
subject_name, message_content, forged=forged, subject_name, message_content, forged=forged,
forged_timestamp = request.POST.get('time'), forged_timestamp = request.POST.get('time'),
forwarder_user_profile=user_profile, realm=realm, forwarder_user_profile=user_profile, realm=realm,
local_id=local_id, sender_queue_id=queue_id) client_message_id=client_message_id, sender_queue_id=queue_id)
return json_success({"id": ret}) return json_success({"id": ret})
def fill_edit_history_entries(message_history, message): def fill_edit_history_entries(message_history, message):