Skip to content

Commit

Permalink
Add connect(), disconnect(), deprecate setAutoReconnect(), `res…
Browse files Browse the repository at this point in the history
…ume()`, and `connected` (#9439)

For context on the reasoning behind these changes see: #8745

This PR adds `connect()` and `disconnect()` to `Container` and `IContainer`. These functions will combine to replace the functionality of `Container.setAutoReconnect()` (which is deprecated here). Furthermore, `connect()` is essentially a rename of `Container.resume()`, hence `resume()` has been deprecated as well.

In addition, `IContainer.connected` has been deprecated in favor of `IContainer.connectionState`. To make `IFluidContainer` consistent with these changes, `connectionState` has been added to `IFluidContainer`, and `IFluidContainer.connected` has been deprecated as well.

Follow up items:
1. Add `connect()` and `disconnect()` to `IFluidContainer` and `IContainer`
2. Add tests for the new functionality
3. Update documentation referencing deprecated APIs
4. Explore behavior if `disconnect()` is called while the container is attempting to connect

Closes #9396, #9167, #9744
  • Loading branch information
scottn12 authored Apr 7, 2022
1 parent 867e337 commit 9031425
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 24 deletions.
16 changes: 16 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ There are a few steps you can take to write a good change note and avoid needing
- [IDirectory extends IDisposable](#IDirectory-extends-IDisposable)
- [raiseContainerWarning removed from IContainerContext](#raiseContainerWarning-removed-from-IContainerContext)
- [`IContainerRuntimeBase.setFlushMode` is deprecated](#icontainerruntimebasesetflushmode-is-deprecated)
- [connected deprecated from IContainer, IFluidContainer, and FluidContainer](#connected-deprecated-from-IContainer-IFluidContainer-and-FluidContainer)
- [setAutoReconnect and resume deprecated from IContainer and Container](#setAutoReconnect-and-resume-deprecated-from-IContainer-and-Container)
- [IContainer.connect() and IContainer.disconnect() will be made mandatory in future major release](#icontainer-connect-and-icontainer-disconnect-will-be-made-mandatory-in-future-major-release)

### Doing operations not allowed on deleted sub directory
Users will not be allowed to do operations on a deleted directory. Users can subscribe to `disposed` event to know if a sub directory is deleted. Accessing deleted sub directory will throw `UsageError` exception now.
Expand All @@ -34,6 +37,19 @@ IDirectory has started extending IDisposable. This means that users implementing
### `IContainerRuntimeBase.setFlushMode` is deprecated
`IContainerRuntimeBase.setFlushMode` is deprecated and will be removed in a future release. FlushMode will become an immutable property for the container runtime, optionally provided at creation time via the `IContainerRuntimeOptions` interface. See [#9480](https://github.com/microsoft/FluidFramework/issues/9480#issuecomment-1084790977)

### connected deprecated from IContainer, IFluidContainer, and FluidContainer
`connected` has been deprecated from `IContainer`, `IFluidContainer`, and `FluidContainer`. It will be removed in a future major release. Use `connectionState` property on the respective interfaces/classes instead. Please switch to the new APIs as soon as possible, and provide any feedback to the FluidFramework team if necessary.
``` diff
- if (fluidContainer.connected)
+ if (fluidContainer.connectionState === ConnectionState.Connected)
```

### setAutoReconnect and resume deprecated from IContainer and Container
`setAutoReconnect()` and `resume()` have been deprecated from `IContainer` and `Container`. They will be removed in a future major release. Use `connect()` instead of `setAutoReconnect(true)` and `resume()`, and use `disconnect()` instead of `setAutoReconnect(false)`. Note, when using these new functions you will need to ensure that the container is both attached and not closed to prevent an error being thrown. Please switch to the new APIs as soon as possible, and provide any feedback to the FluidFramework team if necessary.

### IContainer.connect() and IContainer.disconnect() will be made mandatory in future major release
In major release 1.0, the optional functions `IContainer.connect()` `IContainer.disconnect()` will be made mandatory functions.

## 0.58 Breaking changes
- [Move IntervalType from merge-tree to sequence package](#Move-IntervalType-from-merge-tree-to-sequence-package)
- [Remove logger property from IContainerContext](#Remove-logger-property-from-IContainerContext)
Expand Down
8 changes: 6 additions & 2 deletions api-report/container-loader.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,16 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
// (undocumented)
get closeSignal(): AbortSignal;
// (undocumented)
connect(): void;
// (undocumented)
get connected(): boolean;
// (undocumented)
get connectionState(): ConnectionState;
static createDetached(loader: Loader, codeDetails: IFluidCodeDetails): Promise<Container>;
// (undocumented)
get deltaManager(): IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>;
// (undocumented)
disconnect(): void;
forceReadonly(readonly: boolean): void;
// (undocumented)
getAbsoluteUrl(relativeUrl: string): Promise<string | undefined>;
Expand All @@ -100,13 +104,13 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
request(path: IRequest): Promise<IResponse>;
// (undocumented)
get resolvedUrl(): IResolvedUrl | undefined;
// (undocumented)
// @deprecated
resume(): void;
get scopes(): string[] | undefined;
// (undocumented)
serialize(): string;
get serviceConfiguration(): IClientConfiguration | undefined;
// (undocumented)
// @deprecated
setAutoReconnect(reconnect: boolean): void;
// (undocumented)
get storage(): IDocumentStorageService;
Expand Down
4 changes: 4 additions & 0 deletions api-report/fluid-static.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { AttachState } from '@fluidframework/container-definitions';
import { BaseContainerRuntimeFactory } from '@fluidframework/aqueduct';
import { ConnectionState } from '@fluidframework/container-definitions';
import { DataObject } from '@fluidframework/aqueduct';
import { IAudience } from '@fluidframework/container-definitions';
import { IChannelFactory } from '@fluidframework/datastore-definitions';
Expand Down Expand Up @@ -42,6 +43,7 @@ export class FluidContainer extends TypedEventEmitter<IFluidContainerEvents> imp
attach(): Promise<string>;
get attachState(): AttachState;
get connected(): boolean;
get connectionState(): ConnectionState;
create<T extends IFluidLoadable>(objectClass: LoadableObjectClass<T>): Promise<T>;
dispose(): void;
get disposed(): boolean;
Expand All @@ -59,7 +61,9 @@ export interface IConnection {
export interface IFluidContainer extends IEventProvider<IFluidContainerEvents> {
attach(): Promise<string>;
readonly attachState: AttachState;
// @deprecated
readonly connected: boolean;
readonly connectionState: ConnectionState;
create<T extends IFluidLoadable>(objectClass: LoadableObjectClass<T>): Promise<T>;
dispose(): void;
readonly disposed: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,12 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
close(error?: ICriticalContainerError): void;
closeAndGetPendingLocalState(): string;
readonly closed: boolean;
connect?(): void;
// @deprecated
readonly connected: boolean;
readonly connectionState: ConnectionState;
deltaManager: IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>;
disconnect?(): void;
// @alpha
forceReadonly?(readonly: boolean): any;
getAbsoluteUrl(relativeUrl: string): Promise<string | undefined>;
Expand All @@ -148,10 +151,10 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
readonly readOnlyInfo: ReadOnlyInfo;
request(request: IRequest): Promise<IResponse>;
resolvedUrl: IResolvedUrl | undefined;
// @alpha
// @deprecated
resume?(): void;
serialize(): string;
// @alpha
// @deprecated
setAutoReconnect?(reconnect: boolean): void;
}

Expand Down
21 changes: 19 additions & 2 deletions common/lib/container-definitions/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,20 +252,37 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout

/**
* Boolean indicating whether the container is currently connected or not
* @deprecated - 0.58, This API will be removed in 1.0
* Check `connectionState === ConnectionState.Connected` instead
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
readonly connected: boolean;

/**
* Attempts to connect the container to the delta stream and process ops
*/
connect?(): void;

/**
* Disconnects the container from the delta stream and stops processing ops
*/
disconnect?(): void;

/**
* Dictates whether or not the current container will automatically attempt to reconnect to the delta stream
* after receiving a disconnect event
* @param reconnect - Boolean indicating if reconnect should automatically occur
* @alpha
* @deprecated - 0.58, This API will be removed in 1.0
* Use `connect()` and `disconnect()` instead of `setAutoReconnect(true)` and `setAutoReconnect(false)` respectively
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
setAutoReconnect?(reconnect: boolean): void;

/**
* Have the container attempt to resume processing ops
* @alpha
* @deprecated - 0.58, This API will be removed in 1.0
* Use `connect()` instead
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
resume?(): void;

Expand Down
13 changes: 11 additions & 2 deletions packages/framework/fluid-static/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@
},
"typeValidation": {
"version": "0.58.3000",
"broken": {}
"broken": {
"0.58.2000": {
"ClassDeclaration_FluidContainer": {
"forwardCompat": false
},
"InterfaceDeclaration_IFluidContainer": {
"forwardCompat": false
}
}
}
}
}
}
17 changes: 16 additions & 1 deletion packages/framework/fluid-static/src/fluidContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { TypedEventEmitter } from "@fluidframework/common-utils";
import { IFluidLoadable } from "@fluidframework/core-interfaces";
import { IEvent, IEventProvider } from "@fluidframework/common-definitions";
import { AttachState, IContainer } from "@fluidframework/container-definitions";
import { AttachState, IContainer, ConnectionState } from "@fluidframework/container-definitions";
import { LoadableObjectClass, LoadableObjectRecord } from "./types";
import { RootDataObject } from "./rootDataObject";

Expand Down Expand Up @@ -74,9 +74,17 @@ export interface IFluidContainerEvents extends IEvent {
export interface IFluidContainer extends IEventProvider<IFluidContainerEvents> {
/**
* Whether the container is connected to the collaboration session.
* @deprecated - 0.58, This API will be removed in 1.0
* Check `connectionState === ConnectionState.Connected` instead
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
readonly connected: boolean;

/**
* Provides the current connected state of the container
*/
readonly connectionState: ConnectionState;

/**
* A container is considered **dirty** if it has local changes that have not yet been acknowledged by the service.
* You should always check the `isDirty` flag before closing the container or navigating away from the page.
Expand Down Expand Up @@ -184,6 +192,13 @@ export class FluidContainer extends TypedEventEmitter<IFluidContainerEvents> imp
return this.container.connected;
}

/**
* {@inheritDoc IFluidContainer.connectionState}
*/
public get connectionState(): ConnectionState {
return this.container.connectionState;
}

/**
* {@inheritDoc IFluidContainer.initialObjects}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ declare function get_old_ClassDeclaration_FluidContainer():
declare function use_current_ClassDeclaration_FluidContainer(
use: TypeOnly<current.FluidContainer>);
use_current_ClassDeclaration_FluidContainer(
// @ts-expect-error compatibility expected to be broken
get_old_ClassDeclaration_FluidContainer());

/*
Expand Down Expand Up @@ -144,6 +145,7 @@ declare function get_old_InterfaceDeclaration_IFluidContainer():
declare function use_current_InterfaceDeclaration_IFluidContainer(
use: TypeOnly<current.IFluidContainer>);
use_current_InterfaceDeclaration_IFluidContainer(
// @ts-expect-error compatibility expected to be broken
get_old_InterfaceDeclaration_IFluidContainer());

/*
Expand Down
83 changes: 68 additions & 15 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,6 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i

private resumedOpProcessingAfterLoad = false;
private firstConnection = true;
private manualReconnectInProgress = false;
private readonly connectionTransitionTimes: number[] = [];
private messageCountAfterDisconnection: number = 0;
private _loadedFromVersion: IVersion | undefined;
Expand Down Expand Up @@ -909,11 +908,31 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
);
}

/**
* Dictates whether or not the current container will automatically attempt to reconnect to the delta stream
* after receiving a disconnect event
* @param reconnect - Boolean indicating if reconnect should automatically occur
* @deprecated - 0.58, This API will be removed in 1.0
* Use `connect()` and `disconnect()` instead of `setAutoReconnect(true)` and `setAutoReconnect(false)` respectively
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
public setAutoReconnect(reconnect: boolean) {
if (this.closed) {
throw new Error("Attempting to setAutoReconnect() a closed Container");
}

const mode = reconnect ? ReconnectMode.Enabled : ReconnectMode.Disabled;
this.setAutoReconnectInternal(mode);

// If container state is not attached and resumed, then don't connect to delta stream. Also don't set the
// manual reconnection flag to true as we haven't made the initial connection yet.
if (reconnect && this._attachState === AttachState.Attached && this.resumedOpProcessingAfterLoad) {
// Ensure connection to web socket
this.connectToDeltaStream({ reason: "autoReconnect" });
}
}

private setAutoReconnectInternal(mode: ReconnectMode) {
const currentMode = this._deltaManager.connectionManager.reconnectMode;

if (currentMode === mode) {
Expand All @@ -925,27 +944,65 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
this.setAutoReconnectTime = now;

this.mc.logger.sendTelemetryEvent({
eventName: reconnect ? "AutoReconnectEnabled" : "AutoReconnectDisabled",
eventName: mode === ReconnectMode.Enabled ? "AutoReconnectEnabled" : "AutoReconnectDisabled",
connectionMode: this.connectionMode,
connectionState: ConnectionState[this.connectionState],
duration,
});

this._deltaManager.connectionManager.setAutoReconnect(mode);
}

// If container state is not attached and resumed, then don't connect to delta stream. Also don't set the
// manual reconnection flag to true as we haven't made the initial connection yet.
if (reconnect && this._attachState === AttachState.Attached && this.resumedOpProcessingAfterLoad) {
if (this.connectionState === ConnectionState.Disconnected) {
// Only track this as a manual reconnection if we are truly the ones kicking it off.
this.manualReconnectInProgress = true;
}
public connect() {
if (this.closed) {
throw new UsageError(`The Container is closed and cannot be connected`);
}
else if (this._attachState !== AttachState.Attached) {
throw new UsageError(`The Container is not attached and cannot be connected`);
}
else if (!this.connected) {
// Note: no need to fetch ops as we do it preemptively as part of DeltaManager.attachOpHandler().
// If there is gap, we will learn about it once connected, but the gap should be small (if any),
// assuming that connect() is called quickly after initial container boot.
this.connectInternal({ reason: "DocumentConnect", fetchOpsFromStorage: false });
}
}

// Ensure connection to web socket
this.connectToDeltaStream({ reason: "autoReconnect" });
private connectInternal(args: IConnectionArgs) {
assert(!this.closed, "Attempting to connect() a closed Container");
assert(this._attachState === AttachState.Attached, "Attempting to connect() a container that is not attached");

// Resume processing ops and connect to delta stream
this.resumeInternal(args);

// Set Auto Reconnect Mode
const mode = ReconnectMode.Enabled;
this.setAutoReconnectInternal(mode);
}

public disconnect() {
if (this.closed) {
throw new UsageError(`The Container is closed and cannot be disconnected`);
}
else {
this.disconnectInternal();
}
}

private disconnectInternal() {
assert(!this.closed, "Attempting to disconnect() a closed Container");

// Set Auto Reconnect Mode
const mode = ReconnectMode.Disabled;
this.setAutoReconnectInternal(mode);
}

/**
* Have the container attempt to resume processing ops
* @deprecated - 0.58, This API will be removed in 1.0
* Use `connect()` instead
* See https://github.com/microsoft/FluidFramework/issues/9167 for context
*/
public resume() {
if (!this.closed) {
// Note: no need to fetch ops as we do it preemptively as part of DeltaManager.attachOpHandler().
Expand Down Expand Up @@ -1491,7 +1548,6 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
});

deltaManager.on("disconnect", (reason: string) => {
this.manualReconnectInProgress = false;
this.collabWindowTracker.stopSequenceNumberUpdate();
this.connectionStateHandler.receivedDisconnectEvent(reason);
});
Expand Down Expand Up @@ -1558,8 +1614,6 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
}
if (this.firstConnection) {
connectionInitiationReason = "InitialConnect";
} else if (this.manualReconnectInProgress) {
connectionInitiationReason = "ManualReconnect";
} else {
connectionInitiationReason = "AutoReconnect";
}
Expand All @@ -1584,7 +1638,6 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i

if (value === ConnectionState.Connected) {
this.firstConnection = false;
this.manualReconnectInProgress = false;
}
}

Expand Down

0 comments on commit 9031425

Please sign in to comment.