Rewrite ViewSchema.checkCompatibility to leverage discrepancies #23192

Abe27342 · 2024-11-22T23:07:21Z

Description

Updates ViewSchema.checkCompatibility to leverage the getFieldDiscrepancies codepath.

This also puts in place infrastructure to allow viewing the document when the only differences between view and stored schema are extra optional fields in the stored schema on nodes that the view schema author has declared are OK.

…isRepoSuperset

…edContentDiscrepancies

…AndPolicy, support it in schema validation. Add some import/export tests. Make linter pass.

Abe27342 · 2024-11-22T23:13:12Z

It's probably feasible to split optional field work into a different PR and make this one more about using the new compatibility logic. I can do that if this space is difficult for the reviewer to reason about, or if we're concerned about risk of this change.

I am aware of a few ways that the new compatibility logic is not totally equivalent to the old one, but only places where it is stricter and probably doesn't matter in practice (e.g. it doesn't detect all types of never fields as equivalent, but realistic customers probably aren't going around and declaring such things anyway).

Another thing which we used to allow but now do not is upgrading from an object node schema to a map node schema over the union of all the object fields' types. This is probably not something people are particularly yearning for, and TBH I'm not totally convinced we have the test coverage which verifies this actually works beyond it being theoretically sound.

One thing to watch out for is that a lot of our test coverage for 'the new code path' being equivalent to 'the old code path' comes from implementing an isRepoSuperset based on getFieldDiscrepancies and asserting it is equivalent to the previous compatibility logic's allowsRepoSuperset. Because the new compatibility check no longer directly uses isRepoSuperset, this coverage doesn't directly apply. As such, I wrote a test suite explicitly for ViewSchema which should cover what we need (but reviewer: please verify this!)

…oving it can come later.

packages/dds/tree/src/simple-tree/objectNode.ts

packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts

packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts

msfluid-bot

Code Coverage Summary

↓ packages.dds.tree.src.simple-tree.api:
Line Coverage Change: 0.21% Branch Coverage Change: -0.83%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	89.77%	88.94%	↓ -0.83%
Line Coverage	82.35%	82.56%	↑ 0.21%

↓ packages.dds.tree.src.core.schema-view:
Line Coverage Change: -0.46% Branch Coverage Change: No change

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	100.00%	100.00%	→ No change
Line Coverage	96.80%	96.34%	↓ -0.46%

↓ packages.dds.tree.src.simple-tree:
Line Coverage Change: 0.03% Branch Coverage Change: -0.10%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	94.24%	94.14%	↓ -0.10%
Line Coverage	97.23%	97.26%	↑ 0.03%

↓ packages.dds.tree.src.shared-tree:
Line Coverage Change: -0.01% Branch Coverage Change: No change

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	91.70%	91.70%	→ No change
Line Coverage	97.16%	97.15%	↓ -0.01%

↑ packages.dds.tree.src.core.schema-stored:
Line Coverage Change: 0.01% Branch Coverage Change: No change

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	93.61%	93.61%	→ No change
Line Coverage	99.70%	99.71%	↑ 0.01%

↑ packages.dds.tree.src.feature-libraries:
Line Coverage Change: 0.02% Branch Coverage Change: 0.03%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	94.08%	94.11%	↑ 0.03%
Line Coverage	97.79%	97.81%	↑ 0.02%

↑ packages.dds.tree.src.feature-libraries.modular-schema:
Line Coverage Change: 0.06% Branch Coverage Change: 0.01%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	93.29%	93.30%	↑ 0.01%
Line Coverage	95.14%	95.20%	↑ 0.06%

↑ packages.dds.tree.src.feature-libraries.default-schema:
Line Coverage Change: No change Branch Coverage Change: 0.11%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	93.16%	93.27%	↑ 0.11%
Line Coverage	98.56%	98.56%	→ No change

Baseline commit: abde76d
Baseline build: 310021
Happy Coding!!

Code coverage comparison check passed!!

msfluid-bot · 2024-11-25T19:39:01Z

⯅ @fluid-example/bundle-size-tests: +6.31 KB

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	467.08 KB	467.11 KB	⯅ +35 Bytes
azureClient.js	563.85 KB	563.9 KB	⯅ +49 Bytes
connectionState.js	724 Bytes	724 Bytes	■ No change
containerRuntime.js	263.27 KB	263.29 KB	⯅ +14 Bytes
fluidFramework.js	428.71 KB	431.76 KB	⯅ +3.05 KB
loader.js	134.18 KB	134.19 KB	⯅ +14 Bytes
map.js	42.71 KB	42.71 KB	⯅ +7 Bytes
matrix.js	150.15 KB	150.16 KB	⯅ +7 Bytes
odspClient.js	530.45 KB	530.5 KB	⯅ +49 Bytes
odspDriver.js	98.65 KB	98.67 KB	⯅ +21 Bytes
odspPrefetchSnapshot.js	43.04 KB	43.05 KB	⯅ +14 Bytes
sharedString.js	166.23 KB	166.24 KB	⯅ +7 Bytes
sharedTree.js	419.17 KB	422.21 KB	⯅ +3.04 KB
Total Size	3.38 MB	3.39 MB	⯅ +6.31 KB

Baseline commit: abde76d

Generated by 🚫 dangerJS against 06eb65f

packages/dds/tree/src/simple-tree/api/schemaFactory.ts

packages/dds/tree/src/simple-tree/toStoredSchema.ts

packages/dds/tree/src/feature-libraries/modular-schema/discrepancies.ts

packages/dds/tree/src/simple-tree/api/view.ts

noencke · 2024-12-11T22:20:24Z

packages/dds/tree/src/test/schemaFactoryAlpha.ts

+ * Copy of {@link SchemaFactory} with additional alpha APIs.
+ *
+ * @privateRemarks
+ * Not currently exported to the public API surface as doing so produces errors in API-extractor.


What are the errors? Do you mean "problems with API extractor itself" or "it's not actually properly typed for export yet, and naturally API extractor complains"?

There are some other features that have been thrown around lately that would be implemented in a SchemaFactoryAlpha, so this will need to get lifted/shared eventually.

Yeah. I don't remember exactly what goes wrong, but agreed we'll need to sort this out. I'm planning on lifting this explicitly directly after this PR, so we'll revisit this at that point in a PR that is more scoped :)

github-actions · 2024-12-12T22:48:19Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170006 links
    1595 destination URLs
    1825 URLs ignored
       0 warnings
       0 errors

## Description Exposes `SchemaFactoryAlpha` and associated additional options on `object` and `objectRecursive` to tree's alpha API surface. This lights up the feature implemented in #23192, so I have also added a changeset with some user-facing guidance on that feature. --------- Co-authored-by: Abram Sanderson <absander@microsoft.com>

Abram Sanderson added 16 commits November 5, 2024 16:56

fix(tree): Resolve several inconsistencies in allowsRepoSuperset and …

7783946

…isRepoSuperset

fix lint

7abb5ee

PR feedback

4db405d

chore(tree): Simplify getAllowedContentIncompatibilities output

6d95109

refactor(tree): rename getAllowedContentIncompatibilities to getAllow…

8f74d00

…edContentDiscrepancies

Adjust missed rename

8367272

chore(tree): Convert getAllowedContentDiscrepancies to be a generator

14532f7

Refactor discrepancies.ts to use generators

cdae388

Rewrite ViewSchema.checkCompatibility

764d9d2

Implement one way to deal with neverFields and tests

596ca76

Make allowing optional fields specifiable policy

2f88971

fix lint

09c020a

merge main

b0e5ce7

post-merge lint fixes

926bec6

Make comment better

3eb6e67

Move policy to allow additional optional fields to delegate on Schema…

0c16f31

…AndPolicy, support it in schema validation. Add some import/export tests. Make linter pass.

Abe27342 requested a review from a team as a code owner November 22, 2024 23:07

github-actions bot added base: main PRs targeted against main branch area: dds Issues related to distributed data structures area: dds: tree public api change Changes to a public API labels Nov 22, 2024

Export + import logic from discrepancies.ts rather than copy-paste; m…

d8bb31e

…oving it can come later.

Abe27342 commented Nov 22, 2024

View reviewed changes

packages/dds/tree/src/simple-tree/objectNode.ts Outdated Show resolved Hide resolved

Abe27342 commented Nov 22, 2024

View reviewed changes

packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts Outdated Show resolved Hide resolved

Abe27342 commented Nov 22, 2024

View reviewed changes

packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts Outdated Show resolved Hide resolved

Abram Sanderson added 2 commits November 25, 2024 10:44

fix tree-cli-app's tests

63b83e9

Update fluid-framework api report

cb7e520

github-actions bot added area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct labels Nov 25, 2024

msfluid-bot reviewed Nov 25, 2024

View reviewed changes

Abram Sanderson added 3 commits November 25, 2024 15:49

Add some tests for mapTreeFromNodeData

03440cd

Add test for toMapTree with relevant behavior

0501451

fix lint

06eb65f

noencke approved these changes Dec 11, 2024

View reviewed changes

noencke reviewed Dec 11, 2024

View reviewed changes

Abram Sanderson added 2 commits December 12, 2024 09:42

Merge main and fix merge conflicts

9886887

PR feedback

8ef39c2

Abe27342 enabled auto-merge (squash) December 12, 2024 23:27

Abe27342 merged commit 195b722 into microsoft:main Dec 12, 2024
30 checks passed

Abe27342 deleted the rewrite-checkCompatibility branch December 12, 2024 23:31

Abe27342 mentioned this pull request Dec 18, 2024

Expose SchemaFactoryAlpha as an alpha API #23362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite ViewSchema.checkCompatibility to leverage discrepancies #23192

Rewrite ViewSchema.checkCompatibility to leverage discrepancies #23192

Abe27342 commented Nov 22, 2024

Abe27342 commented Nov 22, 2024 •

edited

Loading

msfluid-bot left a comment •

edited

Loading

msfluid-bot commented Nov 25, 2024 •

edited

Loading

noencke Dec 11, 2024

noencke Dec 11, 2024

Abe27342 Dec 12, 2024

github-actions bot commented Dec 12, 2024

Rewrite ViewSchema.checkCompatibility to leverage discrepancies #23192

Rewrite ViewSchema.checkCompatibility to leverage discrepancies #23192

Conversation

Abe27342 commented Nov 22, 2024

Description

Abe27342 commented Nov 22, 2024 • edited Loading

msfluid-bot left a comment • edited Loading

Choose a reason for hiding this comment

Code Coverage Summary

Code coverage comparison check passed!!

msfluid-bot commented Nov 25, 2024 • edited Loading

noencke Dec 11, 2024

Choose a reason for hiding this comment

noencke Dec 11, 2024

Choose a reason for hiding this comment

Abe27342 Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 12, 2024

linkcheck output

Abe27342 commented Nov 22, 2024 •

edited

Loading

msfluid-bot left a comment •

edited

Loading

msfluid-bot commented Nov 25, 2024 •

edited

Loading