Skip to content

Commit 8facae9

Browse files
committed
centralize peer sync response delivery
1 parent d4e2071 commit 8facae9

5 files changed

Lines changed: 53 additions & 39 deletions

File tree

src/services/peerJsManager.js

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ import {
146146
createSyncResponseForRequest,
147147
findRepoConversation,
148148
getNormalizedSyncResponseMessages,
149+
getSyncResponseDeliveryType,
149150
isValidSyncChainRequestMessage,
150151
isValidSyncRequestMessage,
151152
isValidSyncResponseMessage
@@ -1046,25 +1047,12 @@ function handleSyncRequest(msg, fromPeerId) {
10461047
return;
10471048
}
10481049

1049-
// Get conversation messages
1050-
const conversationsMap = get(conversations);
1051-
const conversation = findRepoConversation(conversationsMap, repoFullName, msg.conversationId);
1052-
1053-
const response = createSyncResponseForRequest(msg, conversation);
1054-
if (response.error === 'Conversation not found') {
1055-
console.warn('[PeerJS] Conversation not found:', msg.conversationId);
1056-
sendMessageToPeer(fromPeerId, response);
1057-
return;
1058-
}
1059-
1060-
if (response.type === 'sync_needs_chain') {
1061-
console.warn('[PeerJS] Hash not found in conversation:', msg.lastHash);
1062-
sendMessageToPeer(fromPeerId, response);
1063-
return;
1064-
}
1065-
1066-
console.log('[PeerJS] Sending', response.messages.length, 'messages after hash:', msg.lastHash);
1067-
sendMessageToPeer(fromPeerId, response);
1050+
const response = createSyncResponseForRequest(msg, getSyncConversation(msg.conversationId));
1051+
sendSyncResponse(fromPeerId, response, {
1052+
conversation_not_found: () => console.warn('[PeerJS] Conversation not found:', msg.conversationId),
1053+
sync_needs_chain: () => console.warn('[PeerJS] Hash not found in conversation:', msg.lastHash),
1054+
messages: () => console.log('[PeerJS] Sending', response.messages.length, 'messages after hash:', msg.lastHash)
1055+
});
10681056
}
10691057

10701058
// Handle sync request with hash chain
@@ -1076,25 +1064,22 @@ function handleSyncRequestWithChain(msg, fromPeerId) {
10761064
return;
10771065
}
10781066

1079-
// Get conversation messages
1080-
const conversationsMap = get(conversations);
1081-
const conversation = findRepoConversation(conversationsMap, repoFullName, msg.conversationId);
1082-
1083-
const response = createSyncResponseForChainRequest(msg, conversation);
1084-
if (response.error === 'Conversation not found') {
1085-
console.warn('[PeerJS] Conversation not found:', msg.conversationId);
1086-
sendMessageToPeer(fromPeerId, response);
1087-
return;
1088-
}
1067+
const response = createSyncResponseForChainRequest(msg, getSyncConversation(msg.conversationId));
1068+
sendSyncResponse(fromPeerId, response, {
1069+
conversation_not_found: () => console.warn('[PeerJS] Conversation not found:', msg.conversationId),
1070+
full_sync: () => console.warn('[PeerJS] No common ancestor found with peer'),
1071+
messages: () => console.log('[PeerJS] Found common ancestor:', response.commonAncestor, 'sending', response.messages.length, 'messages')
1072+
});
1073+
}
10891074

1090-
if (response.fullSync) {
1091-
console.warn('[PeerJS] No common ancestor found with peer');
1092-
sendMessageToPeer(fromPeerId, response);
1093-
return;
1094-
}
1075+
function getSyncConversation(conversationId) {
1076+
return findRepoConversation(get(conversations), repoFullName, conversationId);
1077+
}
10951078

1096-
console.log('[PeerJS] Found common ancestor:', response.commonAncestor, 'sending', response.messages.length, 'messages');
1097-
sendMessageToPeer(fromPeerId, response);
1079+
function sendSyncResponse(peerId, response, deliveryHandlers) {
1080+
const deliveryType = getSyncResponseDeliveryType(response);
1081+
deliveryHandlers[deliveryType]?.();
1082+
sendMessageToPeer(peerId, response);
10981083
}
10991084

11001085
// Handle sync response

src/utils/peerSync.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,19 @@ export function getNormalizedSyncResponseMessages(message) {
137137

138138
return normalizeSyncMessages(message.messages);
139139
}
140+
141+
export function getSyncResponseDeliveryType(response) {
142+
if (response?.error === 'Conversation not found') {
143+
return 'conversation_not_found';
144+
}
145+
146+
if (response?.type === 'sync_needs_chain') {
147+
return 'sync_needs_chain';
148+
}
149+
150+
if (response?.fullSync) {
151+
return 'full_sync';
152+
}
153+
154+
return 'messages';
155+
}

test/e2e/api/peer-sync.spec.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
createSyncResponseFromHashChain,
1212
findRepoConversation,
1313
getNormalizedSyncResponseMessages,
14+
getSyncResponseDeliveryType,
1415
isValidSyncChainRequestMessage,
1516
isValidSyncRequestMessage,
1617
isValidSyncResponseMessage,
@@ -169,3 +170,10 @@ test('getNormalizedSyncResponseMessages validates and normalizes response payloa
169170
{ conversationId: 'conversation-a', messages: [{ sender: 'alice', content: 'hello' }] }
170171
)).toHaveLength(1);
171172
});
173+
174+
test('getSyncResponseDeliveryType classifies sync responses for peer delivery logs', () => {
175+
expect(getSyncResponseDeliveryType({ error: 'Conversation not found' })).toBe('conversation_not_found');
176+
expect(getSyncResponseDeliveryType({ type: 'sync_needs_chain' })).toBe('sync_needs_chain');
177+
expect(getSyncResponseDeliveryType({ fullSync: true })).toBe('full_sync');
178+
expect(getSyncResponseDeliveryType({ messages: [] })).toBe('messages');
179+
});

test/e2e/api/sidebar-chats-source.spec.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,16 @@ test('peerJsManager delegates sync protocol shaping to utilities', async () => {
257257
expect(source).toContain('isValidSyncRequestMessage(msg)');
258258
expect(source).toContain('isValidSyncChainRequestMessage(msg)');
259259
expect(source).toContain('isValidSyncResponseMessage(msg)');
260-
expect(source).toContain('findRepoConversation(conversationsMap, repoFullName, msg.conversationId)');
261-
expect(source).toContain('createSyncResponseForRequest(msg, conversation)');
262-
expect(source).toContain('createSyncResponseForChainRequest(msg, conversation)');
260+
expect(source).toContain('findRepoConversation(get(conversations), repoFullName, conversationId)');
261+
expect(source).toContain('createSyncResponseForRequest(msg, getSyncConversation(msg.conversationId))');
262+
expect(source).toContain('createSyncResponseForChainRequest(msg, getSyncConversation(msg.conversationId))');
263263
expect(source).toContain('getNormalizedSyncResponseMessages(msg)');
264+
expect(source).toContain('getSyncResponseDeliveryType(response)');
265+
expect(source).toContain('sendSyncResponse(fromPeerId, response');
264266
expect(utilitySource).toContain('export function createSyncResponseAfterHash');
265267
expect(utilitySource).toContain('export function createSyncResponseForRequest');
266268
expect(utilitySource).toContain('export function createSyncChainRequestForNeed');
269+
expect(utilitySource).toContain('export function getSyncResponseDeliveryType');
267270
expect(utilitySource).toContain('export function normalizeSyncMessages');
268271
expect(source).not.toContain('getRecentHashes');
269272
expect(source).not.toContain('findCommonAncestor');

test/e2e/ui/login.spec.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ test('renders the login screen for a fresh browser session', async ({ page }) =>
8181
expect(consoleMessages.some(message => message.includes('Switched back to camera'))).toBe(false);
8282
expect(consoleMessages.some(message => message.includes('Started screen sharing'))).toBe(false);
8383
expect(consoleMessages.some(message => message.includes('No common ancestor found with peer'))).toBe(false);
84+
expect(consoleMessages.some(message => message.includes('Invalid sync chain request format'))).toBe(false);
85+
expect(consoleMessages.some(message => message.includes('Found common ancestor'))).toBe(false);
8486
expect(consoleMessages.some(message => message.includes('Notified leader of conversation update'))).toBe(false);
8587
expect(consoleMessages.some(message => message.includes('Received committed messages notification'))).toBe(false);
8688
expect(consoleMessages.some(message => message.includes('Requesting sync with hash chain'))).toBe(false);

0 commit comments

Comments
 (0)