api: In send-message requests, override user agent for old servers

This resolves zulip#440 for older servers.
chrisbobbe · Jan 17, 2024 · 5363be5 · 5363be5
1 parent ec54546
commit 5363be5
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 5 deletions.
diff --git a/lib/api/core.dart b/lib/api/core.dart
@@ -74,13 +74,16 @@ class ApiConnection {
   bool _isOpen = true;
 
   Future<T> send<T>(String routeName, T Function(Map<String, dynamic>) fromJson,
-      http.BaseRequest request) async {
+      http.BaseRequest request, {String? overrideUserAgent}) async {
     assert(_isOpen);
 
     assert(debugLog("${request.method} ${request.url}"));
 
     addAuth(request);
     request.headers.addAll(userAgentHeader());
+    if (overrideUserAgent != null) {
+      request.headers['User-Agent'] = overrideUserAgent;
+    }
 
     final http.StreamedResponse response;
     try {
@@ -137,13 +140,13 @@ class ApiConnection {
   }
 
   Future<T> post<T>(String routeName, T Function(Map<String, dynamic>) fromJson,
-      String path, Map<String, dynamic>? params) async {
+      String path, Map<String, dynamic>? params, {String? overrideUserAgent}) async {
     final url = realmUrl.replace(path: "/api/v1/$path");
     final request = http.Request('POST', url);
     if (params != null) {
       request.bodyFields = encodeParameters(params)!;
     }
-    return send(routeName, fromJson, request);
+    return send(routeName, fromJson, request, overrideUserAgent: overrideUserAgent);
   }
 
   Future<T> postFileFromStream<T>(String routeName, T Function(Map<String, dynamic>) fromJson,

diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart
@@ -180,6 +180,7 @@ Future<SendMessageResult> sendMessage(
   bool? readBySender,
 }) {
   final supportsTypeDirect = connection.zulipFeatureLevel! >= 174; // TODO(server-7)
+  final supportsReadBySender = connection.zulipFeatureLevel! >= 236; // TODO(server-8)
   return connection.post('sendMessage', SendMessageResult.fromJson, 'messages', {
     if (destination is StreamDestination) ...{
       'type': RawParameter('stream'),
@@ -195,6 +196,23 @@ Future<SendMessageResult> sendMessage(
     if (queueId != null) 'queue_id': queueId, // TODO should this use RawParameter?
     if (localId != null) 'local_id': localId, // TODO should this use RawParameter?
     if (readBySender != null) 'read_by_sender': readBySender,
+  },
+  overrideUserAgent: switch ((supportsReadBySender, readBySender)) {
+    // Old servers use the user agent to decide if we're a UI client
+    // and so whether the message should be marked as read for its author
+    // (see #440). We are a UI client; so, use a value those servers will
+    // interpret correctly. With newer servers, passing `readBySender: true`
+    // gives the same result.
+    // TODO(#467) include platform, platform version, and app version
+    (false, _   ) => 'ZulipMobile/flutter',
+
+    // According to the doc, a user-agent heuristic is still used in this case:
+    //   https://zulip.com/api/send-message#parameter-read_by_sender
+    // TODO find out if our default user agent would work with that.
+    // TODO(#467) include platform, platform version, and app version
+    (true,  null) => 'ZulipMobile/flutter',
+
+    _             => null,
   });
 }
 

diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart
@@ -3,6 +3,7 @@ import 'dart:convert';
 import 'package:checks/checks.dart';
 import 'package:http/http.dart' as http;
 import 'package:test/scaffolding.dart';
+import 'package:zulip/api/core.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/model/narrow.dart';
 import 'package:zulip/api/route/messages.dart';
@@ -304,6 +305,7 @@ void main() {
       String? localId,
       bool? readBySender,
       required Map<String, String> expectedBodyFields,
+      String? expectedUserAgent,
     }) async {
       connection.prepare(json: SendMessageResult(id: 42).toJson());
       final result = await sendMessage(connection,
@@ -313,7 +315,8 @@ void main() {
       check(connection.lastRequest).isA<http.Request>()
         ..method.equals('POST')
         ..url.path.equals('/api/v1/messages')
-        ..bodyFields.deepEquals(expectedBodyFields);
+        ..bodyFields.deepEquals(expectedBodyFields)
+        ..headers['User-Agent'].equals(expectedUserAgent ?? userAgentHeader()['User-Agent']!);
     }
 
     test('smoke', () {
@@ -374,7 +377,39 @@ void main() {
             'to': jsonEncode(userIds),
             'content': content,
             'read_by_sender': 'true',
-          });
+          },
+          expectedUserAgent: 'ZulipMobile/flutter');
+      });
+    });
+
+    test('when readBySender is null, sends a User-Agent we know the server will recognize', () {
+      return FakeApiConnection.with_((connection) async {
+        await checkSendMessage(connection,
+          destination: StreamDestination(streamId, topic), content: content,
+          expectedBodyFields: {
+            'type': 'stream',
+            'to': streamId.toString(),
+            'topic': topic,
+            'content': content,
+          },
+          readBySender: null,
+          expectedUserAgent: 'ZulipMobile/flutter');
+      });
+    });
+
+    test('legacy: when server does not support readBySender, sends a User-Agent the server will recognize', () {
+      return FakeApiConnection.with_(zulipFeatureLevel: 235, (connection) async {
+        await checkSendMessage(connection,
+          destination: StreamDestination(streamId, topic), content: content,
+          expectedBodyFields: {
+            'type': 'stream',
+            'to': streamId.toString(),
+            'topic': topic,
+            'content': content,
+            'read_by_sender': 'true',
+          },
+          readBySender: true,
+          expectedUserAgent: 'ZulipMobile/flutter');
       });
     });
   });