-
Notifications
You must be signed in to change notification settings - Fork 554
(compat) Added supported features and generation across Loader / Driver boundary #24462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(compat) Added supported features and generation across Loader / Driver boundary #24462
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds compatibility validation logic and tests between the Loader and Driver layers by introducing new properties such as supported features and generation. Key changes include:
- Adding new tests and validation logic for layer compatibility in both Loader and Driver contexts.
- Refactoring and updating code to use the new test utilities (e.g., createTestCodeLoader) and compatibility state functions.
- Exporting and integrating new driver support requirements and compatibility details into the Loader layer.
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/test/test-end-to-end-tests/src/test/layerCompat.spec.ts | Added end-to-end tests to validate Loader/Driver compatibility behavior. |
packages/loader/container-loader/src/test/testCodeLoader.ts | Introduced a test code loader with updated compatibility behavior. |
packages/loader/container-loader/src/test/loaderLayerCompatValidation.spec.ts | Expanded tests to verify version and feature compatibility for both Runtime and Driver layers. |
packages/loader/container-loader/src/test/loader.spec.ts | Updated tests to use the new test code loader and ensure integration with compatibility validation. |
packages/loader/container-loader/src/loaderLayerCompatState.ts | Added and exported the driver support requirements and compatibility validation for the Driver layer. |
packages/loader/container-loader/src/container.ts | Updated the setter for the document service to perform Driver compatibility checks before assignment. |
packages/drivers/local-driver/src/localRuntimeLayerCompatState.ts, localDocumentService.ts, and index.ts | Added and exposed compatibility details from the local Driver to the Loader layer. |
Files not reviewed (3)
- packages/drivers/local-driver/package.json: Language not supported
- packages/test/test-end-to-end-tests/package.json: Language not supported
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/loader/container-loader/src/container.ts:602
- The identifier 'FluidObject' is used without an import, which may lead to a ReferenceError. Please ensure that FluidObject is imported from the appropriate module (e.g., '@fluidframework/core-interfaces').
const maybeDriverCompatDetails = service as FluidObject<ILayerCompatDetails>;
bc0896b
to
bb373bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
56964cd
to
423425e
Compare
…er boundary - Part 2 (#24855) ## Reviewer Guidance Follow up to #24462 which added the logic to local driver and the validation login in the Loader layer. This PR extends the same logic to other drivers as well. Also, note that there are a couple of test-only document service implementations to which this logic is not added - `FileDocumentService` and `StaticStorageDocumentService`. These drivers should not care about layer compatibility for now. If they do need this, it can be added later on a case-by-case basic. Keeping the logic on a need basis to keep things simple. ## Description Added logic to support layer compatibility validation at the Loader / Driver layer boundary. Note that the validation only happens in the Loader layer and not in the Driver layer. This is because Driver layer doesn't call into any objects in the Loader layer. So, it doesn't need to support compat in that direction. For context, please look at #22877 which added the same support to the Runtime / Loader boundary. This logic added by this PR is the same as in #22877. ### Supported features - The `Driver` layer adds a `supportedFeatures` property to the `document service` that includes a set of features it supports. This is advertised to the Loader layer at the layer boundary. - The `Loader` layer has a `requiredFeatures` array that includes a set of features that it requires the driver layer to support in order to be compatible. Note that this is internal and is not advertised. - At runtime, the `Loader` layer validates that all the entries in `requiredFeatures` is present in the `supportedFeatures` of the `Driver`. If not, it throws a usage error that says layers are not compatible. - Any change that adds new behavior or changes existing behavior across this layer boundary must add an entry to the supported features set of the `Driver`. For example, changes such as "does the driver support this new summary format foo" should add an entry to the supported features set of the driver. ### Generation - In addition to `supported features`, a `generation` is also added to the `Driver` layer. This is advertised to the `Loader` layer at the layer boundary. - `Generation` is an integer which will be incremented every month. It can be used to validate compatibility more strictly (time-based) than `supported features` where layers are incompatible only if there are unsupported features. _Note: The logic to update the generation will be added in a later PR. See [AB#27054](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/27054) for a proposed solution._ - One key reason for adding generation is to have a clear cut off point where we officially stop supporting layer combinations. Our compat tests will test combinations up to this cut off point. The idea is that we test what we support and whatever we don't test should not be supported. This will help us achieve that. - The `Loader` layer has a `minSupportedGeneration` that it needs the `Driver` layer to be at. Note that this is internal and is not advertised. - At runtime, the `Loader` layer validates that the `generation` of the `Driver` layer is `>= minSupportedGeneration`. - For example, say that the `Loader` is at generation 20 and has a `minSupportedGeneration` of 8 for the `Driver` layer (12 month support window). If it sees that `Driver`'s generation is lower than 8, the layers are incompatible. ## Backwards compatibility To support older layers before layer compat enforcement is introduced, the `minSupportedGeneration` is set to 0 whereas `generation` starts at 1. For older layers that did not implement `ILayerCompatDetails`, their generation is set to 0 and `supported features` is set to an empty set. So, their `generation` is compatible until we update the `minSupportedGeneration`. [AB#20807](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/20807)
Reviewer Guidance
Currently, this PR added the logic to local driver only. If this looks good, I will add it to other drivers as well.
I considered adding this to
document service factory
so that the validation can be done inContainer
's constructor. However, document service seems like the better object because it haspolicies
which serve a similar purpose but is more permanent.Description
Added logic to support layer compatibility validation at the Loader / Driver layer boundary. Note that the validation only happens in the Loader layer and not in the Driver layer. This is because Driver layer doesn't call into any objects in the Loader layer. So, it doesn't need to support compat in that direction.
For context, please look at #22877 which added the same support to the Runtime / Loader boundary. This logic added by this PR is the same as in #22877.
Supported features
Driver
layer adds asupportedFeatures
property to thedocument service
that includes a set of features it supports. This is advertised to the Loader layer at the layer boundary.Loader
layer has arequiredFeatures
array that includes a set of features that it requires the driver layer to support in order to be compatible. Note that this is internal and is not advertised.Loader
layer validates that all the entries inrequiredFeatures
is present in thesupportedFeatures
of theDriver
. If not, it throws a usage error that says layers are not compatible.Driver
. For example, changes such as "does the driver support this new summary format foo" should add an entry to the supported features set of the driver.Generation
supported features
, ageneration
is also added to theDriver
layer. This is advertised to theLoader
layer at the layer boundary.Generation
is an integer which will be incremented every month. It can be used to validate compatibility more strictly (time-based) thansupported features
where layers are incompatible only if there are unsupported features.Note: The logic to update the generation will be added in a later PR. See AB#27054 for a proposed solution.
Loader
layer has aminSupportedGeneration
that it needs theDriver
layer to be at. Note that this is internal and is not advertised.Loader
layer validates that thegeneration
of theDriver
layer is>= minSupportedGeneration
.Loader
is at generation 20 and has aminSupportedGeneration
of 8 for theDriver
layer (12 month support window). If it sees thatDriver
's generation is lower than 8, the layers are incompatible.Backwards compatibility
To support older layers before layer compat enforcement is introduced, the
minSupportedGeneration
is set to 0 whereasgeneration
starts at 1. For older layers that did not implementILayerCompatDetails
, their generation is set to 0 andsupported features
is set to an empty set. So, theirgeneration
is compatible until we update theminSupportedGeneration
.AB#20807