enable single commit summary via supportedFeatures in routerlicious

[pragya91]

Enable single-commit summaries via summarizeProtocolTree policy flag in IDocumentServicePolicies - which for routerlicious will be set when enable_single_commit_summary flag is provided in the supportedFeatures property bag in IConnected. This change is particularly aimed at enabling slow rollouts of FRS clients on the single-commit summary flow.

1. updated routerlicious-driver's documentService to check the supported features and set the summarizeProtocolTree driver policy. Container uses this flag to check whether or not the protocol tree should be added to the summary before sending the summary.
2. Updated containerStorageAdapter to always initialize ProtocolTreeStorageService.
3. ProtocolTreeStorageService uses a callback to check whether it should upload protocoltree with summary or not. This is done to ensure we are using the most updated value of driver policy which will be set correctly only when the delta connection is established.
4. Renamed forceEnableSummarizeProtocolTree to shouldSummarizeProtocolTree.

AB#7864

[msfluid-bot]

⯅ @fluid-example/bundle-size-tests: +508 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	453.34 KB	453.34 KB	■ No change
azureClient.js	552.55 KB	552.83 KB	⯅ +284 Bytes
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	257.02 KB	257.02 KB	■ No change
fluidFramework.js	359.88 KB	359.88 KB	■ No change
loader.js	134.36 KB	134.46 KB	⯅ +112 Bytes
map.js	41.53 KB	41.53 KB	■ No change
matrix.js	143.75 KB	143.75 KB	■ No change
odspClient.js	520.91 KB	521.02 KB	⯅ +112 Bytes
odspDriver.js	97.3 KB	97.3 KB	■ No change
odspPrefetchSnapshot.js	42.16 KB	42.16 KB	■ No change
sharedString.js	160.27 KB	160.27 KB	■ No change
sharedTree.js	359.86 KB	359.86 KB	■ No change
Total Size	3.2 MB	3.2 MB	⯅ +508 Bytes

---

Baseline commit: 7cba8d9

Generated by 🚫 dangerJS against bd7b408

[jatgarg]

Why are we inverting the logic here? I think we should only wrap inside the ProtocolTreeStorageService if we want to summarize protocol tree and not check inside the ProtocolTreeStorageService if it is enabled or not.

[pragya91]

This new change is implemented so that we query for the service policies when needed. This is will give drivers enough time to establish socket connection, so that the connection's supportedFeatures can be used if needed. Just like in this PR.

[vladsud]

not clear why you do ?? {} if return type allows undefined

[vladsud]

This cast is not great. Anything we can do to make it more type-safe?

[vladsud]

I'd suggest to always update policy, on every connection. I.e. allow true -> false transitions - service might want to pull back the flight.

[vladsud]

It looks like a bug (that your code is fixing), but worth doing some more digging to make sure we are not missing something (and not changing behavior that we do not understand):
The code before did not use retriableStorage if this.summarizeProtocolTree === false.
Now we use it all the time.

It looks correct, but raising to make sure we do proper due dilligence here.

[vladsud]

if you move logging call into addProtocolSummaryIfMissing(), then this code can be much simpler:

return this.internalStorageService.uploadSummaryWithContext(
   this.shouldSummarizeProtocolTree() ? this.addProtocolSummaryIfMissing(summary) : summary,
   context);

[vladsud]

Would be useful to test that calling connectToDeltaStream()

1. Maintains the status-quo if supportedFeatures keeps returning same result
2. Can reset policy if supportedFeatures changes its flags (on second reconnect)