Skip to content
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

refactor(tree): rename getAllowedContentIncompatibilities to getAllow… #7

Open
wants to merge 87 commits into
base: normalize-to-forbidden
Choose a base branch
from

Conversation

Abe27342
Copy link
Owner

@Abe27342 Abe27342 commented Nov 6, 2024

Description

The general direction this code is moving toward is a two-step process: one function locates differences between the view and stored schema (getAllowedContentDiscrepancies), and another uses that function's output to determine if the view schema is compatible with the stored schema (right now isRepoSuperset in the same file serves that job, but it is likely this bit will be refactored to make more sense with some other layering, so the naming/implementation may change).

I'd like to isolate policy decisions to be outside of code that lives in this first step. To this end, I believe using Discrepancy is a bit more suggestive of the unopinionated nature: the function simply reports differences between two schemas.

sonalideshpandemsft and others added 30 commits November 1, 2024 12:33
…22937)

This PR add a new command to generate node10 type declaration
entrypoints. Most of the node10 code is extracted from [generate
entrypoint](https://github.com/microsoft/FluidFramework/blob/main/build-tools/packages/build-cli/src/library/commands/generateEntrypoints.ts)
command.
fix parameter use in condition. swapping parameter to intermediate
variable.
## Description

Added import/export options for tree content and schema, and example
script using them.
Bumped build-tools from 0.50.0 to 0.51.0.
I wasn't able to reproduce the flakiness locally (ran 2000 times), but
I'm still seeing recent timeout failures in the build pipelines. These
tests aren't testing anything that's driver specific, so it's safe to
skip them for standard r11s.

Fixes
[AB#11213](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/11213)
Fixes
[AB#11060](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/11060)
…rosoft#22967)

There is nothing particularly Fluid-centric about the top-level API in
build-infrastructure; I just couldn't think of a better name at the
time. After discussion, we're going to call the top-level object a
BuildProject. The full hierarchy is now:

BuildProjects contain Workspaces contain ReleaseGroups contain Packages

This change required a lot of documentation updates and renaming other
related properties and functions beyond the BuildProject itself.

The package is private so these changes are safe.
This change adds a `setVersion` API that acts on several packages.


[AB#21855](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/21855)

---------

Co-authored-by: Alex Villarreal <[email protected]>
Co-authored-by: Joshua Smithrud <[email protected]>
## Description

This introduces a new SharedTree branching API that follows the ideas in
[this
document](https://github.com/microsoft/FluidFramework/pull/22740/files#diff-0073fb61eb0ca7c2b1f9bc918f31ed46270c5a104cee8b419dac1d82845167da).
Essentially, the branching API remains the same but has been exposed
directly on the view objects (rather than on a separate object) for the
user's convenience. The previous branching API's entry point is
deprecated.

See the changeset and new documentation for more details.
)

1. `ISessionClient` method names updated for consistency to
`getConnectionId()` and `getConnectionStatus()`.
2. Implementation of `ISessionClient` moved to a full class object.
3. Changeset provided for Presence changes since 2.4.
4. Updated `id` to `ID` in comments (public and most internal).

No behavior is changed.

[AB#21446](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/21446)

---------

Co-authored-by: Willie Habimana <[email protected]>
Co-authored-by: Tyler Butler <[email protected]>
)

Generates the release notes for 2.5. Most changesets have been updated
with minor wording and formatting changes. Command used to generate the
release notes:

```shell
pnpm flub generate releaseNotes -g client -t minor --outFile RELEASE_NOTES/2.5.0.md
```

---------

Co-authored-by: jzaffiro <[email protected]>
## Description

Tag asserts short codes before Minor release

Co-authored-by: Jatin Garg <[email protected]>
…t#22976)

## Description

build(client): changelog generation for minor release 2.5.0

Co-authored-by: Jatin Garg <[email protected]>
## Description

Bump client minor 2.10.0

---------

Co-authored-by: Jatin Garg <[email protected]>
…atement (microsoft#22929)

Updates the primary loop used in the release command to use a map
instead of a switch statement.
…version (microsoft#22966)

Upgrades all the dependencies to their latest compatible version within
the dep range in package.json.
…#22974)

Updated the following:

  client (release group)

Dependencies on build-tools updated:

  @fluid-tools/build-cli: 0.50.0
  @fluidframework/build-tools: 0.50.0
  @fluidframework/bundle-size-tools: 0.50.0
  @fluid-tools/version-tools: 0.50.0
…osoft#22908)

Picks up the latest patch of Biome. Since it's just a patch, I updated
the lockfiles only to save the package.json churn.
…2977)

adds a new `changed` event to `TreeBranchEvents` that fires for both
local and remote changes

---------

Co-authored-by: Noah Encke <[email protected]>
…2986)

## Description

Currently, there are some filesystem operations in Gitrest that result
in a generic 400 HTTP error code, rather than a helpful HTTP status and
message based on the error that occurred.

This PR adds some wrapper functions that help determine if an error is a
FileSystemError (or RedisFSError, which is similar) and bubble that up
as a NetworkError that can be parsed for the HTTP response.
## Description

generic types with an extends clause of `never` currently generate type
tests which do not compile due to any not extending never. This is fixed
by using `never` instead of `any`.

`@system` types which do not promise user facing stability are now
omitted from type testing.

The currently unused tags to opt into alternative type testing for a
given type have been converted from `-` separated to camel case since it
seems `-` does not work well in tags.
…22983)

## Description

build(client): Update typetests after minor release 2.5.0

---------

Co-authored-by: Jatin Garg <[email protected]>
The `Client` class in the merge-tree package has been removed.
Additionally, types that directly or indirectly expose the merge-tree
`Client` class have also been removed.

The removed types were not meant to be used directly, and direct usage
was not supported:

- AttributionPolicy
- IClientEvents
- IMergeTreeAttributionOptions
- SharedSegmentSequence
- SharedStringClass

Some classes that referenced the `Client` class have been transitioned
to interfaces. Direct instantiation of these classes was not supported
or necessary for any supported scenario, so the change to an interface
should not impact usage. This applies to the following types:

- SequenceInterval
- SequenceEvent
- SequenceDeltaEvent
- SequenceMaintenanceEvent

The initial deprecations of the now changed or removed types were
announced in Fluid Framework v2.4.0:
[Several MergeTree Client Legacy APIs are now
deprecated](https://github.com/microsoft/FluidFramework/blob/main/RELEASE_NOTES/2.4.0.md#several-mergetree-client-legacy-apis-are-now-deprecated-22629)

---------

Co-authored-by: Tyler Butler <[email protected]>
…22988)

## Description

Moves the merge semantics doc to user-facing folder.
…2995)

Current pattern ?.?.? doesn't support multi digit versions. Bump to
client 2.10.0 has caused the "Install package with perf tests" step to
be failing in the pipeline.
…icrosoft#22985)

Leverages newer dice roller sample and most up-to-date devtools visuals
Disable these tests against the services where they always fail. This
exercises an experimental feature that is not fully supported yet.
## Description

This is a private package that only runs some tests. It has no need of
production dependencies, so this PR does two things:
- Move all its dependencies to `devDependencies`
- Removes several dependencies that were not used
Abram Sanderson and others added 30 commits November 8, 2024 10:31
…isRepoSuperset (microsoft#22999)

## Description

Fixes several bugs/inconsistencies in `allowsRepoSuperset` and
`isRepoSuperset` and adds unit tests for this behavior.

## Bugs

1. Forbidden fields should not be convertible to required fields, but
`isRepoSuperset` allowed this conversion.
2. The production codepath did not correctly handle queries between
value schema nodes and other types of nodes (without the added code in
`comparison.ts`, one could hit `0x893` directly below)
3. `isRepoSuperset` did not previously permit addition of new node kinds

## Context

`allowsRepoSuperset` is the current implementation used by the
production codepath to check compatibility between a document's stored
and view schema. There is ongoing work on the schema evolution front to
transition this codepath to one which is more flexible / easily
debugged, which is the logic in `discrepancies.ts`. The general
philosophy behind `discrepancies.ts` is to split the compatibility check
into two parts: one bit of code which detects all differences between
view and stored schema, and another bit of code which decides if any of
those differences should prohibit document operations (read, write,
upgrade schema, etc). This PR helps bring the newer implementation to
parity with what we have today.

---------

Co-authored-by: Abram Sanderson <[email protected]>
…oft#23038)

## Description

This changes all internal SharedTree objects to have an `events:
Listenable` property rather than implementing Listenable directly. Using
the `.events` pattern is preferable over the alternatives because it
does not employ inheritance (like extending `EventEmitter`) and does not
require any method implementation boilerplate (like implementing
`Listenable`). It also means that any changes to `EventEmitter` or
`Listenable` will not require changes to the object emitting the events
- this is the practical motivation for this change, as the `Listenable`
interface will soon be updated to have a new method. Without this
preparatory change, all the implementations of `Listenable` would need
to be updated at that time.
…to recommended base (microsoft#23042)

The `minimal-deprecated` config is scheduled for deletion. This PR
migrates the tree package off of that config.

Fixes some of the simpler violations, but otherwise disables rules in
the local config for future work to address.


[AB#6983](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/6983)
## Description

Add more ImplicitAllowedTypes docs
## Description

More docs for SchemaFactory.scope.

Convert docs to other format for parameter properties so it can have its
own subsections.
…ft#23035)

Reverts microsoft#23010.

Since it wasn't part of the list of removals for 2.10, holding this
until 2.20.
## Description

Fix a link broken by
microsoft#22988.

## Breaking Changes

None
…osoft#22981)

This is a test repro of
[AB#4630](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/4630).
To reproduce such issue, I had to fake the localId generation using
uuidOverride constructor method in the blob manager so we could aim at a
unique pending blob: such pending blob was: 1 stashed, 2 reapplied to a
different blob manager simulating rehydration, 3 processed and 4
reuploaded by stashing workflow, which ends up in the issue described in
the ADO item.
In order to wait for the reupload I had to create a different blob
manager method waitForStashedBlobs() since the one we have
hasPendingStashedUploads() uses pendingBlobs array as reference, which
blob entry would be removed by the processing step.
A fix will follow up this PR.
This change takes the existing logic to load the createNewModule and
encapsulates it in a function to make it more reusable, and less error
prone.

The primary motivation for this change is that in an upcoming change
I'll want to use the createNewModule and don't want to duplicate the
existing logic

By moving the files of the createNewModule under a folder, and not
exporting them directly, it makes it clearer that they should not be
directly imported.
…#23011)

This PR fixes how we save the obliterate sequence number on the
associated `pendingSegmentGroup`. Previously, the sequence number would
never be set for zero-length obliterates, so we would get stuck
considering the obliterate as unsequenced.

This PR also surfaces zero-length obliterates in
`mergeTreeDeltaCallback` as long as the obliterate's endpoints are not
equal.
picocolors is a smaller alternative to chalk, and it is also ESM and
CJS, while chalk went ESM-only and we were stuck on old versions in the
core build-tools project.

build-tools will eventually be ESM, and using a dual-module-format
package will actually make that transition easier because we won't have
to do dependency switches while also doing ESM conversion.
## Description

This exposes the `off()` method on the `Listenable` event interface,
providing an additional way to unsubscribe from events. The current way
of unsubscribing, via the deregistration function returned by `on()`, is
still supported. Having both options available is meant to give
customers options without being opinionated about the best pattern, as
it depends on preference and use case.

To prevent the two strategies from interfering with each other, it has
also been made illegal to register the same listener more than once for
the same event. It is now documented as throwing an error rather than as
undefined behavior.

This also adds a test for registering events with keys that are JS
symbols and does some minor doc cleanup.
…icrosoft#23062)

## Description

Currently only exporting SessionClientStatus, this change exports both
the type and the constant values
Disabling no-unchecked-record-access for every client package that is
erroring when enabling this

[AB#11815](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/11815)
[AB#15222](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/15222)

Improve summarizer documentation
- SummaryTreeBuilder documentation
- Other summary cleanup

---------

Co-authored-by: Craig Macomber (Microsoft) <[email protected]>
Co-authored-by: Joshua Smithrud <[email protected]>
…in ODSP Driver (microsoft#23060)

## Description


[AB#22192](https://dev.azure.com/fluidframework/internal/_workitems/edit/22192)

This fixes a bug reported by Loop where the reconnecting banner did not
go away after user returned after being dormant for some time.

Reason for the bug:
Due to socket reuse(multiplexing), we can get "disconnect" event from
other clients in the socket reference. So, a race condition could
happen, where this client is establishing connection and listening for
"connect_document_success" on the socket among other events, but we get
"disconnect" event on the socket reference from other clients, in which
case, we dispose connection object and stop listening to further events
on the socket. Due to this we get stuck as the connection is not yet
established and so we don't return any connection object to the client
(connection manager). So, we remain stuck.
In order to handle this, listen for the "disconnect" event and reject
the promise with the error so that the caller can know and handle the
error.

---------

Co-authored-by: Jatin Garg <[email protected]>
…osoft#23041)

Previous output allowed undefined in FieldKindIncompatibility instances
where either the stored or view schema did not have some field
implicitly. However, it is also possible to declare a field as not
present explicitly using forbidden, which should be treated identically
when considering compatibility. This updates the output representation
as well as logic to normalize implicit cases to use forbidden.

---------

Co-authored-by: Abram Sanderson <[email protected]>
microsoft#23069)

Moves fluid-experimental/ai-collab package to the fluidframework/ai-collab scope.
## Description

It turns out the reason I couldn't allow constructing recursive maps
from objects previously could be overcome my inlining the types. I don't
have a complete understanding of how inlineing type sometimes allows
recursive types to work better, but in this case it worked.
…plexing in ODSP Driver" (microsoft#23078)

Reverts microsoft#23060

We're seeing a huge increase in errors in our odsp tests against both
prod and dogfood starting with the first run after this PR was merged,
so reverting as the probable culprit until we can investigate further.
…oft#23076)

## Description

This moves `innerNode` into `#hydrationState`, which makes it clearer
what the possible states and transitions are. A side effect of this is
that the `innerNode` property is now `undefined` in the case where a
node has been hydrated by its corresponding flex node has not yet been
generated. Previously, the `innerNode` remained an
`UnhydratedFlexTreeNode` even after hydration, which while not incorrect
was still confusing to see in the debugger. This PR also does some
related cleanups. However, it does not attempt to clean up
TreeNodeKernel entirely - there is still plenty more possible cleanup,
but this PR would like to avoid trying to do everything at once.
## Description

Improve chunk comments
…nge (microsoft#23059)

This change splits out the changes the PropertiesManger class from
microsoft#22751. The goal is to supply a smaller set of changes that are easier
to review, and allow microsoft#22751 to be checked in in two parts. After this
all that will be left in microsoft#22751 is plumbing the feature through the
layers.

For the broader context of this change, please read the descriptions
from: microsoft#22751

---------

Co-authored-by: Tyler Butler <[email protected]>
Co-authored-by: Abram Sanderson <[email protected]>
…s extension (microsoft#23081)

Also moves the "source of truth" of the images to our Azure blob storage
to reduce the number of binary files in our repo.

### Updated assets:

Logo:
![](https://storage.fluidframework.com/static/images/devtools/logo.png)

Large marquee:

![](https://storage.fluidframework.com/static/images/devtools/marquee-large.png)

Small marquee:

![](https://storage.fluidframework.com/static/images/devtools/marquee-small.png)

These updated assets have been uploaded to the Chrome and Edge stores
and will be visible to users after our next "deployment".
…2842)

Fixes:
[AB#2309](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/2309)

## Description

In light of [Remove ability to reject reentrant ops (microsoft#20621) ·
microsoft/FluidFramework@c9d1562
(github.com)](microsoft@c9d1562),
updating this to be about finishing the removal of
ensureNoDataModelChanges.

The following uses of ensureNoDataModelChanges will be removed:

- packages\runtime\container-runtime\src\channelCollection.ts:
    - ensureNoDataModelChanges: (...args) => {

- packages\runtime\container-runtime\src\dataStoreContext.ts:
    - public ensureNoDataModelChanges(callback: () => T): T {

- packages\runtime\runtime-definitions\src\dataStoreContext.ts:
   - ensureNoDataModelChanges<T>(callback: () => T): T;

- packages\runtime\test-runtime-utils\src\mocksDataStoreContext.ts:
    - public ensureNoDataModelChanges(callback: () => T): T {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.