odspDocumentDeltaConnection targetClientId check

[WillieHabi]

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.

[msfluid-bot]

⯅ @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: cf339a9

Generated by 🚫 dangerJS against 6b59bf1

[WillieHabi]

Rearranged the documentId check as well as added an assert for when documentId is not defined but targetClientId is @jason-ha

[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?

[GaryWilber]

@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

[WillieHabi]

@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

[jason-ha]

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.

[msfluid-bot]

Code Coverage Summary

↓ packages.drivers.odsp-driver.src:
Line Coverage Change: No change    Branch Coverage Change: -0.29%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	85.72%	85.43%	↓ -0.29%
Line Coverage	74.93%	74.93%	→ No change

---

Baseline commit: cf339a9
Baseline build: 301282
Happy Coding!!

Code coverage comparison check passed!!

[jason-ha]

Why was this complication added?

[WillieHabi]

Signal layer compat tests fails without this, specifically the layer version combination where you have a very old loader and the most recent driver layer. Old loader doesn't handle batched signals (ISignalMessage[]) so only individual ISignalMessage's should be passed when there's one element for backcompat,