Add odsp driver support to upload container type summary

[jatgarg]

Fixes: #8079
This adds support for odsp driver to upload container type summary which includes the ".protocol" summary too along with app summary.
Currently there is no support at server to handle the full summary, so we get the SummaryNack in case we do upload the full summary in odsp driver.
But this is behind feature gate "SummarizeProtocolTree" in loader layer, so I think we don't need to add another feature gate for driver as if loader is sending the protocol tree then in that case odsp driver does have to upload everything, so I think another feature gate might not make sense. Let me know your thoughts @anthony-murphy .

[msfluid-bot]

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

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	382.38 KB	382.38 KB	■ No change
containerRuntime.js	189.49 KB	189.49 KB	■ No change
loader.js	159.99 KB	159.99 KB	■ No change
map.js	204.59 KB	204.59 KB	■ No change
matrix.js	292.09 KB	292.09 KB	■ No change
odspDriver.js	183.09 KB	183.21 KB	⯅ +115 Bytes
odspPrefetchSnapshot.js	76.73 KB	76.73 KB	■ No change
sharedString.js	311.57 KB	311.57 KB	■ No change
Total Size	1.79 MB	1.79 MB	⯅ +115 Bytes

---

Baseline commit: 5681a58

Generated by 🚫 dangerJS against 533325d

[GaryWilber]

Regarding Currently there is no support at server to handle the full summary, so we get the SummaryNack in case we do upload the full summary in odsp driver.
I will need to make a change on my side to support this. Is it possible to have the client include a new flag in the summary op to indicate that it uploaded the container tree? There is an includesProtocolTree flag set today - maybe another flag can be added, such as isContainerTree

[anthony-murphy]

Regarding Currently there is no support at server to handle the full summary, so we get the SummaryNack in case we do upload the full summary in odsp driver. I will need to make a change on my side to support this. Is it possible to have the client include a new flag in the summary op to indicate that it uploaded the container tree? There is an includesProtocolTree flag set today - maybe another flag can be added, such as isContainerTree

supporting a big matrix of unshipped features is not great. i'm fine if we just switch the server behavior. just let use know when you assume includesProtocolTree means container upload and we'll make the switch too. maybe do a server side flag, and we'll just coordinate rollout as none of this is in prod.

[GaryWilber]

supporting a big matrix of unshipped features is not great. i'm fine if we just switch the server behavior. just let use know when you assume includesProtocolTree means container upload and we'll make the switch too. maybe do a server side flag, and we'll just coordinate rollout as none of this is in prod.

The issue I'm facing is that the server needs to know what kind of tree the client uploaded.

There are a total of 3 cases:

1. Client uploads a channel tree without a .protocol tree in it
2. Client uploads a channel tree with a .protocol tree
3. Client uploads a container tree with a .protocol tree

The server has to run different logic depending on each case:

1. Server commits the summary by POSTing a container, which is a server generated .protocol tree + the client created .app tree
2. Server fetches the client created .protocol tree, verifies the .protocol tree is valid, and POSTs that channel tree as a container.
3. Server should not do anything (no calls to ODSP) because the client already did the commit.

The server submits a summaryAck after processing the summary in each case.

The server knows case 1 and 2 are different because of the includesProtocolTree flag.

However the server cannot determine the difference between case 2 and 3 because the flag is set for both. This means the server will have to figure out if the client uploaded a channel or container tree by calling ODSP. So "case 3" will actually involve a call to ODSP now, which is working against the purpose of what we want - I don't want Push to have to call ODSP when the client already committed the summary.

[jatgarg]

supporting a big matrix of unshipped features is not great. i'm fine if we just switch the server behavior. just let use know when you assume includesProtocolTree means container upload and we'll make the switch too. maybe do a server side flag, and we'll just coordinate rollout as none of this is in prod.

The issue I'm facing is that the server needs to know what kind of tree the client uploaded.

There are a total of 3 cases:

1. Client uploads a channel tree without a .protocol tree in it
2. Client uploads a channel tree with a .protocol tree
3. Client uploads a container tree with a .protocol tree

The server has to run different logic depending on each case:

1. Server commits the summary by POSTing a container, which is a server generated .protocol tree + the client created .app tree
2. Server fetches the client created .protocol tree, verifies the .protocol tree is valid, and POSTs that channel tree as a container.
3. Server should not do anything (no calls to ODSP) because the client already did the commit.

The server submits a summaryAck after processing the summary in each case.

The server knows case 1 and 2 are different because of the includesProtocolTree flag.

However the server cannot determine the difference between case 2 and 3 because the flag is set for both. This means the server will have to figure out if the client uploaded a channel or container tree by calling ODSP. So "case 3" will actually involve a call to ODSP now, which is working against the purpose of what we want - I don't want Push to have to call ODSP when the client already committed the summary.

Yes, I think that is what Tony is saying. For now, even for 3rd you can do same as you do in 2 because it will not be worse than the current state. Then once we are confident, w can enable the flag permanently and then you can stop checking the type and eliminate 2nd state.