microsoft
odspDocumentDeltaConnection targetClientId check
ODSP driver reuses sockets for clients connected on the same node process. When signals are received on the shared socket, we must filter based on targetClientId property to make sure only the specified client receives the signal.
⯅ @fluid-example/bundle-size-tests: +476 Bytes
| Metric Name | Baseline Size | Compare Size | Size Diff |
|---|---|---|---|
| aqueduct.js | 459.8 KB | 459.84 KB | ⯅ +35 Bytes |
| azureClient.js | 556.94 KB | 556.99 KB | ⯅ +49 Bytes |
| connectionState.js | 724 Bytes | 724 Bytes | ■ No change |
| containerRuntime.js | 259.42 KB | 259.43 KB | ⯅ +14 Bytes |
| fluidFramework.js | 405.81 KB | 405.82 KB | ⯅ +14 Bytes |
| loader.js | 134.16 KB | 134.18 KB | ⯅ +14 Bytes |
| map.js | 42.46 KB | 42.46 KB | ⯅ +7 Bytes |
| matrix.js | 148.29 KB | 148.29 KB | ⯅ +7 Bytes |
| odspClient.js | 523.91 KB | 523.95 KB | ⯅ +49 Bytes |
| odspDriver.js | 97.84 KB | 97.86 KB | ⯅ +21 Bytes |
| odspPrefetchSnapshot.js | 42.81 KB | 42.82 KB | ⯅ +14 Bytes |
| sharedString.js | 164.48 KB | 164.49 KB | ⯅ +7 Bytes |
| sharedTree.js | 396.27 KB | 396.28 KB | ⯅ +7 Bytes |
| Total Size | 3.31 MB | 3.31 MB | ⯅ +476 Bytes |
Baseline commit: cf339a9872180ae457049c6c010f64097a8e152c
Generated by :no_entry_sign: dangerJS against 6b59bf12f576802ffc1b2aef598a353682566c51
Rearranged the documentId check as well as added an assert for when documentId is not defined but targetClientId is @jason-ha
@GaryWilber, does the assert added for when there is no documentId look correct? That no signal message in that case would have a targetClientId?
The documentId param is always included so it should never hit that
@GaryWilber, does the assert added for when there is no documentId look correct? That no signal message in that case would have a targetClientId?
The documentId param is always included so it should never hit that
Ok thanks. Since documentId is always included, we should then assert that documentId is defined(?)
Was there an original reason why we continued to send signal when !documentId? @GaryWilber
More tests are preferred in this PR. But if not then add a tech debt work item to follow up. Coverage is needed for multiplexing on and off as well as multiple documents and especially targeted and not targeted signals.
