Skip to content

Commit 7d7551a

Browse files
xsbfhxsbfh
authored andcommitted
implement meta delivered/received status for split-parts messages
Signed-off-by: xsbfh <z6MkpzZDoFM7tPZPXqvaVCtMFxKJQAxghVRJZxsf8s8SqBvm@dev.smashchats.com>
1 parent 4724191 commit 7d7551a

9 files changed

Lines changed: 132 additions & 17 deletions

File tree

TODO

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,15 @@
2020
- [x] clearing another user from the neighborhood
2121

2222
- [x] embedded media type
23-
- [x] message parts
23+
- [x] message parts & limit message queue size to keep websocket alive
24+
- [ ] emit sent and delivered status when all parts have been ack'd as such
25+
26+
- [ ] Parts feedback to Client to allow them to restore queue ??? **OR RATHER**, think through the state restore & queue recovery –> half-a, half-b, full
27+
- [ ] add tests for concurrent messages re-assembling
28+
- [ ] add tests for out of order parts
29+
- [ ] test persistence and resilience (save/load state between restarts)
30+
31+
- [ ] **Investigate INITCHATS: new Date() or new Date(0)–why?? FIX DOCS.**
2432

2533
- [ ] livestream embedded media (snap) (before sharing decryption key at last, then reconstitute) – PARTS IN PARTS??? or binary only for content???
2634

@@ -62,6 +70,10 @@
6270
- [ ] investigate performance and memory usage (eg, dangling sessions, dlqs, parts, no redundant compute)
6371
- [ ] Use UTF8 vs Base64 for raw content encoding??
6472

73+
WARNINGS/DISCLAIMERS
74+
75+
[Warning] AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like AES-GCM to protect against chosen-ciphertext attacks. (2key-ratchet.js, line 6011)
76+
6577
### TESTS REFACTORING (keeping or increasing coverage):
6678

6779
- [ ] generate unit .test files with mocks/spies for each module corresponding to the library API reference (**testing only publicly exported**) --> should have 100% coverage for publicly exposed APIs (public and internal)

src/api/SmashMessaging.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { PeerRegistry } from '@src/core/messaging/peer/PeerRegistry.js';
77
import { SmashPeer } from '@src/core/messaging/peer/SmashPeer.js';
88
import { MessageMiddleware } from '@src/core/messaging/protocol/MessageMiddleware.js';
99
import { MessageReassembler } from '@src/core/messaging/protocol/MessageReassembler.js';
10+
import { MessageSplitter } from '@src/core/messaging/protocol/MessageSplitter.js';
1011
import { SessionManager } from '@src/core/messaging/session/SessionManager.js';
1112
import { EndpointManager } from '@src/infrastructure/network/endpoints/EndpointManager.js';
1213
import { SMESocketManager } from '@src/infrastructure/network/sme/SMESocketManager.js';
@@ -28,6 +29,7 @@ import type {
2829
IDIDResolver,
2930
IIMPeerIdentity,
3031
IMProtoMessage,
32+
IMReceivedACKMessage,
3133
reverseDNS,
3234
sha256,
3335
} from '@src/shared/types/index.js';
@@ -36,6 +38,9 @@ import {
3638
IMProfile,
3739
IMReadACKMessage,
3840
MessageStatus,
41+
MessageStatusDelivered,
42+
MessageStatusRead,
43+
MessageStatusReceived,
3944
} from '@src/shared/types/messages/index.js';
4045
import { LogLevel, Logger } from '@src/shared/utils/Logger.js';
4146
import { EventEmitter } from 'events';
@@ -50,6 +55,7 @@ export class SmashMessaging extends EventEmitter {
5055
protected readonly peers: PeerRegistry;
5156
private readonly sessionManager: SessionManager;
5257
private readonly smeSocketManager: SMESocketManager;
58+
private readonly messageSplitter: MessageSplitter;
5359
private readonly messageReassembler: MessageReassembler;
5460
private meta: Partial<IMProfile> = {};
5561

@@ -66,9 +72,11 @@ export class SmashMessaging extends EventEmitter {
6672
) {
6773
super();
6874
this.logger = new Logger(logId, logLevel);
75+
const messageStatusHandler = this.messagesStatusHandler.bind(this);
76+
6977
this.smeSocketManager = new SMESocketManager(
7078
this.logger,
71-
this.messagesStatusHandler.bind(this),
79+
messageStatusHandler,
7280
);
7381

7482
this.peers = new PeerRegistry(
@@ -96,6 +104,10 @@ export class SmashMessaging extends EventEmitter {
96104
);
97105

98106
this.messageReassembler = new MessageReassembler(this.logger);
107+
this.messageSplitter = new MessageSplitter(
108+
this.logger,
109+
messageStatusHandler,
110+
);
99111

100112
this.registerEventHandlers();
101113
this.logger.info(
@@ -194,6 +206,7 @@ export class SmashMessaging extends EventEmitter {
194206
lastMessageTime,
195207
this.sessionManager,
196208
this.smeSocketManager,
209+
this.messageSplitter,
197210
this.peers,
198211
);
199212
}
@@ -252,6 +265,12 @@ export class SmashMessaging extends EventEmitter {
252265
`Successfully reassembled message ${reassembledMessage.sha256} from parts`,
253266
);
254267
await this.notifyNewMessage(sender, reassembledMessage);
268+
(await this.peers.getOrCreate(sender)).send(
269+
await encapsulateMessage({
270+
type: IM_ACK_RECEIVED,
271+
data: [reassembledMessage.sha256],
272+
} as IMReceivedACKMessage),
273+
);
255274
} else {
256275
this.logger.debug(
257276
`Waiting for more parts for message ${(message.data as IMPartData).originalSha256}`,
@@ -284,11 +303,11 @@ export class SmashMessaging extends EventEmitter {
284303
};
285304

286305
this.on(IM_ACK_RECEIVED, (from, message) => {
287-
handleAck('received', from, message.data);
306+
handleAck(MessageStatusReceived, from, message.data);
288307
});
289308

290309
this.on(IM_ACK_READ, (from, message) => {
291-
handleAck('read', from, message.data);
310+
handleAck(MessageStatusRead, from, message.data);
292311
});
293312
}
294313

@@ -300,6 +319,9 @@ export class SmashMessaging extends EventEmitter {
300319
`messagesStatusHandler ACK:${status} : ${messageIds.join(', ')}`,
301320
);
302321
this.emit('status', status, messageIds);
322+
if (status === MessageStatusDelivered) {
323+
this.messageSplitter.markAsDelivered(messageIds);
324+
}
303325
}
304326

305327
/**

src/core/messaging/peer/SmashPeer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export class SmashPeer {
5454
private readonly lastMessageTime: number,
5555
private readonly sessionManager: SessionManager,
5656
private readonly smeSocketManager: SMESocketManager,
57+
private readonly messageSplitter: MessageSplitter,
5758
private readonly peerRegistry: PeerRegistry,
5859
config: PeerConfig = {},
5960
) {
@@ -164,7 +165,7 @@ export class SmashPeer {
164165
const messageSize = MessageSplitter.getMessageSize(encapsulatedMessage);
165166
const queue =
166167
messageSize > MAX_MESSAGE_SIZE
167-
? await MessageSplitter.split(this.logger, encapsulatedMessage)
168+
? await this.messageSplitter.split(encapsulatedMessage)
168169
: [encapsulatedMessage];
169170
return this.mutex.acquire(`flushQueue-${this.id}`, async () => {
170171
for (let i = 0; i < queue.length; i++) {

src/core/messaging/protocol/MessageSplitter.ts

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,36 @@
11
import { encapsulateMessage } from '@src/api/tools/encapsulateMessage.js';
22
import { MAX_MESSAGE_SIZE } from '@src/shared/constants/protocol.js';
33
import { IM_PART } from '@src/shared/lexicon/improto.lexicon.js';
4+
import {
5+
MessageStatusDelivered,
6+
onMessagesStatusFn,
7+
} from '@src/shared/types/index.js';
48
import type {
59
EncapsulatedIMProtoMessage,
610
IMProtoMessage,
711
} from '@src/shared/types/message.types.js';
812
import type { IMPartData } from '@src/shared/types/messages/IMPartMessage.js';
13+
import { sha256 } from '@src/shared/types/string.types.js';
914
import { Logger } from '@src/shared/utils/Logger.js';
1015

16+
interface PartsCounter {
17+
originalSha256: sha256;
18+
counter: number;
19+
}
20+
1121
export class MessageSplitter {
22+
private readonly sending: Map<sha256, PartsCounter> = new Map();
23+
24+
constructor(
25+
private readonly logger: Logger,
26+
private readonly onMessagesStatusCallback: onMessagesStatusFn,
27+
) {}
28+
1229
/**
1330
* Split a message into parts.
1431
* Each part will contain a portion of the original message data.
1532
*/
16-
public static async split(
17-
logger: Logger,
33+
public async split(
1834
message: EncapsulatedIMProtoMessage,
1935
): Promise<EncapsulatedIMProtoMessage[]> {
2036
if (!message?.sha256) {
@@ -24,7 +40,7 @@ export class MessageSplitter {
2440
const messageJson = JSON.stringify(message);
2541
const totalParts = Math.ceil(messageJson.length / MAX_MESSAGE_SIZE);
2642

27-
logger.info(
43+
this.logger.info(
2844
`Splitting message (${messageJson.length} bytes) into ${totalParts} parts`,
2945
);
3046

@@ -43,7 +59,7 @@ export class MessageSplitter {
4359
content: partContent, // No need for base64 encoding
4460
};
4561

46-
logger.debug(
62+
this.logger.debug(
4763
`Created part ${i + 1}/${totalParts} (${partContent.length} bytes)`,
4864
);
4965

@@ -55,6 +71,11 @@ export class MessageSplitter {
5571
}),
5672
);
5773
}
74+
const counter = {
75+
counter: parts.length,
76+
originalSha256: message.sha256,
77+
} as PartsCounter;
78+
parts.forEach((part) => this.sending.set(part.sha256, counter));
5879
return parts;
5980
}
6081

@@ -64,4 +85,32 @@ export class MessageSplitter {
6485
public static getMessageSize(message: IMProtoMessage): number {
6586
return JSON.stringify(message).length;
6687
}
88+
89+
/**
90+
* ACK potential parts as being delivered
91+
* @param parts - Array of message parts to be marked as delivered.
92+
*/
93+
public markAsDelivered(parts: sha256[]) {
94+
if (this.sending.size === 0) return;
95+
this.logger.debug(
96+
`Trying to mark ${parts.length} potential parts as delivered`,
97+
);
98+
parts.forEach((part) => this.markPartAsDelivered(part));
99+
}
100+
101+
private markPartAsDelivered(part: sha256) {
102+
const counter = this.sending.get(part);
103+
if (counter) {
104+
this.logger.debug(`Marking part ${part} as delivered`);
105+
if (--counter.counter === 0) {
106+
this.sending.delete(part);
107+
this.logger.debug(
108+
`All parts have been delivered for ${counter.originalSha256}`,
109+
);
110+
this.onMessagesStatusCallback(MessageStatusDelivered, [
111+
counter.originalSha256,
112+
]);
113+
}
114+
}
115+
}
67116
}

src/infrastructure/network/sme/SMESocketWriteOnly.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { onMessagesStatusFn } from '@src/shared/types/callbacks.types.js';
2+
import { MessageStatusDelivered } from '@src/shared/types/index.js';
23
import type { sha256 } from '@src/shared/types/string.types.js';
34
import type { Logger } from '@src/shared/utils/Logger.js';
45
import { Timeout } from '@src/shared/utils/Timeout.js';
@@ -148,7 +149,10 @@ export class SMESocketWriteOnly {
148149
this.logger.debug(
149150
`> ${buffer.byteLength} bytes (${messageIds.length} messages) sent to ${this.url} [${this.socket?.id}]`,
150151
);
151-
this.onMessagesStatusCallback('delivered', messageIds);
152+
this.onMessagesStatusCallback(
153+
MessageStatusDelivered,
154+
messageIds,
155+
);
152156
resolve();
153157
});
154158
});

src/shared/types/messages/IMAckMessage.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,18 @@ export interface IMReceivedACKMessage extends IMACKMessage {
2020
type: typeof IM_ACK_RECEIVED;
2121
}
2222

23+
export const MessageStatusSending = 'sending' as const;
24+
export const MessageStatusDelivered = 'delivered' as const;
25+
export const MessageStatusReceived = 'received' as const;
26+
export const MessageStatusRead = 'read' as const;
27+
export const MessageStatusError = 'error' as const;
28+
2329
export type MessageStatus =
24-
| 'sending'
25-
| 'delivered'
26-
| 'received'
27-
| 'read'
28-
| 'error';
30+
| typeof MessageStatusSending
31+
| typeof MessageStatusDelivered
32+
| typeof MessageStatusReceived
33+
| typeof MessageStatusRead
34+
| typeof MessageStatusError;
2935
// TODO: double check status lifecycle implementation accross lib
3036
// in particular, error states and (re)sending states
3137

tests/how-to/edgecases.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
IMTextMessage,
1414
IM_CHAT_TEXT,
1515
Logger,
16+
MessageStatusDelivered,
1617
SESSION_TTL_MS,
1718
SmashMessaging,
1819
sha256,
@@ -118,7 +119,7 @@ describe('SmashMessaging: Edge cases', () => {
118119
logger.info('>> Verify Alice received the ACK');
119120
await delay(TEST_CONFIG.MESSAGE_DELIVERY);
120121
expect(alice.onStatus).toHaveBeenCalledWith(
121-
'delivered',
122+
MessageStatusDelivered,
122123
expect.arrayContaining([sent.sha256]),
123124
);
124125

tests/how-to/index.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
IM_CHAT_TEXT,
1010
IM_PROFILE,
1111
Logger,
12+
MessageStatusDelivered,
1213
SmashMessaging,
1314
} from 'smash-node-lib';
1415

@@ -139,7 +140,7 @@ describe('[SmashMessaging] Between peers registered to a SME', () => {
139140
await delay(TEST_CONFIG.DEFAULT_SETUP_DELAY);
140141

141142
expect(alice.onStatus).toHaveBeenCalledWith(
142-
'delivered',
143+
MessageStatusDelivered,
143144
expect.arrayContaining([TEST_MESSAGE.sha256]),
144145
);
145146
},

tests/how-to/parts.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,16 @@ describe('Message Splitting', () => {
8383
const onBobMessage = jest.fn();
8484
bob.messaging.on(IM_CHAT_TEXT, onBobMessage);
8585

86+
const onAliceStatus = jest.fn();
87+
alice.messaging.on('status', onAliceStatus);
88+
8689
const bobReceivedMessage = waitFor(bob.messaging, IM_CHAT_TEXT);
8790

91+
// Alice sends an oversized message to Bob
8892
const sent = await alice.messaging.send(bob.did, message);
8993
await bobReceivedMessage;
9094

95+
// Bob receives the original message and it is correctly handled
9196
expect(onBobMessage).toHaveBeenCalledWith(
9297
alice.did.id,
9398
expect.objectContaining<IMProtoMessage>({
@@ -98,6 +103,20 @@ describe('Message Splitting', () => {
98103
after: sent.after,
99104
}),
100105
);
106+
107+
await delay(TEST_CONFIG.MESSAGE_DELIVERY);
108+
109+
// Alice receives a delivered ACK for the original message
110+
expect(onAliceStatus).toHaveBeenCalledWith(
111+
'delivered',
112+
expect.arrayContaining([sent.sha256]),
113+
);
114+
115+
// Alice receives a received ACK for the original message
116+
expect(onAliceStatus).toHaveBeenCalledWith(
117+
'received',
118+
expect.arrayContaining([sent.sha256]),
119+
);
101120
});
102121

103122
test('Sending a large media message', async () => {

0 commit comments

Comments
 (0)