Skip to content

Commit

Permalink
Remove deprecated container connection APIs (#10192)
Browse files Browse the repository at this point in the history
This PR removes the APIs deprecated in 0.58 via #9439, which include `Container.setAutoReconnect()`, `Container.resume()`, `IContainer.connected`, and `IFluidContainer.connected`. 
Additionally, this PR replaces any remaining usages/references of the above APIs with their respective replacements (see #9167).


Closes #9397
  • Loading branch information
scottn12 authored May 27, 2022
1 parent 7585ff5 commit 3c923a5
Show file tree
Hide file tree
Showing 20 changed files with 72 additions and 127 deletions.
15 changes: 15 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ For more details on GC and options for controlling its behavior, please see [thi
- [`ISummarizerOptions` is deprecated](#isummarizerOptions-is-deprecated)
- [connect() and disconnect() made mandatory on IContainer and IFluidContainer](#connect-and-disconnect-made-mandatory-on-icontainer-and-ifluidcontainer)
- [Remove Const Enums from Merge Tree, Sequence, and Shared String](#Remove-Const-Enums-from-Merge-Tree-Sequence-and-Shared-String)
- [Remove Container.setAutoReconnect() and Container.resume()](#Remove-Container-setAutoReconnect-and-resume)
- [Remove IContainer.connected and IFluidContainer.connected](#Remove-IContainer-connected-and-IFluidContainer-connected)

### Changed AzureConnectionConfig API
- Added a `type` field that's used to differentiate between remote and local connections.
Expand Down Expand Up @@ -103,6 +105,19 @@ export interface IMergeTreeInsertMsg extends IMergeTreeDelta {
+ type: typeof MergeTreeDeltaType.INSERT;
```

### Remove Container.setAutoReconnect() and Container.resume()
The functions `Container.setAutoReconnect()` and `Container.resume()` were deprecated in 0.58 and are now removed. To replace their functionality use `Container.connect()` instead of `Container.setAutoReconnect(true)` and `Container.resume()`, and use `Container.disconnect()` instead of `Container.setAutoReconnect(false)`.

### Remove IContainer.connected and IFluidContainer.connected
The properties `IContainer.connected` and `IFluidContainer.connected` were deprecated in 0.58 and are now removed. To replace their functionality use `IContainer.connectionState` and `IFluidContainer.connectionState` respectively. Example:

``` diff
- if (container.connected) {
+ if (container.connectionState === ConnectionState.Connected) {
console.log("Container is connected");
}
```

# 0.59

## 0.59 Upcoming changes
Expand Down
4 changes: 0 additions & 4 deletions api-report/container-loader.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,10 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
request(path: IRequest): Promise<IResponse>;
// (undocumented)
get resolvedUrl(): IResolvedUrl | undefined;
// @deprecated
resume(): void;
get scopes(): string[] | undefined;
// (undocumented)
serialize(): string;
get serviceConfiguration(): IClientConfiguration | undefined;
// @deprecated
setAutoReconnect(reconnect: boolean): void;
// (undocumented)
get storage(): IDocumentStorageService;
// (undocumented)
Expand Down
3 changes: 0 additions & 3 deletions api-report/fluid-static.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export class FluidContainer extends TypedEventEmitter<IFluidContainerEvents> imp
attach(): Promise<string>;
get attachState(): AttachState;
connect(): Promise<void>;
get connected(): boolean;
get connectionState(): ConnectionState;
create<T extends IFluidLoadable>(objectClass: LoadableObjectClass<T>): Promise<T>;
disconnect(): Promise<void>;
Expand All @@ -64,8 +63,6 @@ export interface IFluidContainer extends IEventProvider<IFluidContainerEvents> {
attach(): Promise<string>;
readonly attachState: AttachState;
connect(): void;
// @deprecated
readonly connected: boolean;
readonly connectionState: ConnectionState;
create<T extends IFluidLoadable>(objectClass: LoadableObjectClass<T>): Promise<T>;
disconnect(): void;
Expand Down
9 changes: 5 additions & 4 deletions api-report/test-utils.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class EventAndErrorTrackingLogger extends TelemetryLogger {
};
// (undocumented)
send(event: ITelemetryBaseEvent): void;
}
}

// @public (undocumented)
export type fluidEntryPoint = SupportedExportInterfaces | IFluidModule;
Expand Down Expand Up @@ -192,7 +192,7 @@ export class LoaderContainerTracker implements IOpProcessingController {
processOutgoing(...containers: IContainer[]): Promise<void>;
reset(): void;
resumeProcessing(...containers: IContainer[]): IContainer[];
}
}

// @public
export class LocalCodeLoader implements ICodeDetailsLoader {
Expand Down Expand Up @@ -291,7 +291,9 @@ export class TestObjectProvider implements ITestObjectProvider {
updateDocumentId(resolvedUrl: IResolvedUrl | undefined): void;
// (undocumented)
get urlResolver(): IUrlResolver;
}
// (undocumented)
waitContainerToCatchUp(container: IContainer): Promise<boolean>;
}

// @public (undocumented)
export function timeoutAwait<T = void>(promise: PromiseLike<T>, timeoutOptions?: TimeoutWithError | TimeoutWithValue<T>): Promise<T>;
Expand Down Expand Up @@ -319,7 +321,6 @@ export interface TimeoutWithValue<T = void> {
value: T;
}


// (No @packageDocumentation comment for this package)

```
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,6 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
closeAndGetPendingLocalState(): string;
readonly closed: boolean;
connect(): void;
// @deprecated
readonly connected: boolean;
readonly connectionState: ConnectionState;
deltaManager: IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>;
disconnect(): void;
Expand All @@ -151,11 +149,7 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
readonly readOnlyInfo: ReadOnlyInfo;
request(request: IRequest): Promise<IResponse>;
resolvedUrl: IResolvedUrl | undefined;
// @deprecated
resume?(): void;
serialize(): string;
// @deprecated
setAutoReconnect?(reconnect: boolean): void;
}

// @public
Expand Down
34 changes: 4 additions & 30 deletions common/lib/container-definitions/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,6 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
*/
readonly connectionState: ConnectionState;

/**
* 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
*/
Expand All @@ -291,32 +283,14 @@ export interface IContainer extends IEventProvider<IContainerEvents>, IFluidRout
*/
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
* @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
* @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;

/**
* The audience information for all clients currently associated with the document in the current session
*/
readonly audience: IAudience;

/**
* The server provided ID of the client.
* Set once this.connected is true, otherwise undefined
* Set once this.connectionState === ConnectionState.Connected is true, otherwise undefined
* @alpha
*/
readonly clientId?: string | undefined;
Expand Down Expand Up @@ -466,12 +440,12 @@ export interface IContainerLoadMode {
| "all";
deltaConnection?:
/*
* Connection to delta stream is made only when Container.resume() call is made. Op processing
* is paused (when container is returned from Loader.resolve()) until Container.resume() call is made.
* Connection to delta stream is made only when Container.connect() call is made. Op processing
* is paused (when container is returned from Loader.resolve()) until Container.connect() call is made.
*/
| "none"
/*
* Connection to delta stream is made only when Container.resume() call is made.
* Connection to delta stream is made only when Container.connect() call is made.
* Op fetching from storage is performed and ops are applied as they come in.
* This is useful option if connection to delta stream is expensive and thus it's beneficial to move it
* out from critical boot sequence, but it's beneficial to allow catch up to happen as fast as possible.
Expand Down
4 changes: 2 additions & 2 deletions examples/hosts/hosts-sample/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/
import { IContainer } from "@fluidframework/container-definitions";
import { Loader } from "@fluidframework/container-loader";
import { ConnectionState, Loader } from "@fluidframework/container-loader";
import {
IUser,
} from "@fluidframework/protocol-definitions";
Expand Down Expand Up @@ -108,7 +108,7 @@ async function start() {
}

// Wait for connection so that proposals can be sent.
if (container !== undefined && !container.connected) {
if (container !== undefined && container.connectionState !== ConnectionState.Connected) {
await new Promise<void>((resolve, reject) => {
// the promise resolves when the connected event fires.
container.once("connected", () => resolve());
Expand Down
4 changes: 2 additions & 2 deletions packages/framework/fluid-framework/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@
"ClassDeclaration_SequenceDeltaEvent": {"forwardCompat": false},
"ClassDeclaration_SharedDirectory": {"forwardCompat": false},
"ClassDeclaration_SharedString": {"forwardCompat": false},
"ClassDeclaration_FluidContainer": {"forwardCompat": false},
"InterfaceDeclaration_IFluidContainer": {"forwardCompat": false},
"ClassDeclaration_FluidContainer": {"forwardCompat": false, "backCompat": false},
"InterfaceDeclaration_IFluidContainer": {"forwardCompat": false, "backCompat": false},
"ClassDeclaration_SequenceEvent": {"forwardCompat": false},
"ClassDeclaration_IntervalCollection": {"forwardCompat": false},
"InterfaceDeclaration_ISharedString": {"forwardCompat": false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ declare function get_current_ClassDeclaration_FluidContainer():
declare function use_old_ClassDeclaration_FluidContainer(
use: TypeOnly<old.FluidContainer>);
use_old_ClassDeclaration_FluidContainer(
// @ts-expect-error compatibility expected to be broken
get_current_ClassDeclaration_FluidContainer());

/*
Expand Down Expand Up @@ -373,6 +374,7 @@ declare function get_current_InterfaceDeclaration_IFluidContainer():
declare function use_old_InterfaceDeclaration_IFluidContainer(
use: TypeOnly<old.IFluidContainer>);
use_old_InterfaceDeclaration_IFluidContainer(
// @ts-expect-error compatibility expected to be broken
get_current_InterfaceDeclaration_IFluidContainer());

/*
Expand Down
3 changes: 2 additions & 1 deletion packages/framework/fluid-static/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
"typeValidation": {
"version": "0.60.1000",
"broken": {
"InterfaceDeclaration_IFluidContainer": {"forwardCompat": false}
"InterfaceDeclaration_IFluidContainer": {"forwardCompat": false, "backCompat": false},
"ClassDeclaration_FluidContainer": {"backCompat": false}
}
}
}
15 changes: 0 additions & 15 deletions packages/framework/fluid-static/src/fluidContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,6 @@ export interface IFluidContainerEvents extends IEvent {
* to the data as well as status on the collaboration session.
*/
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
*/
Expand Down Expand Up @@ -195,13 +187,6 @@ export class FluidContainer extends TypedEventEmitter<IFluidContainerEvents> imp
return this.container.closed;
}

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

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

/*
Expand Down Expand Up @@ -156,6 +157,7 @@ declare function get_current_InterfaceDeclaration_IFluidContainer():
declare function use_old_InterfaceDeclaration_IFluidContainer(
use: TypeOnly<old.IFluidContainer>);
use_old_InterfaceDeclaration_IFluidContainer(
// @ts-expect-error compatibility expected to be broken
get_current_InterfaceDeclaration_IFluidContainer());

/*
Expand Down
2 changes: 1 addition & 1 deletion packages/loader/container-loader/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Container will also not attempt to reconnect on lost connection if `Container.di

Data stores should almost never listen to these events (see more on [Readonly states](#Readonly-states), and should use consensus DDSes if they need to synchronize activity across clients. DDSes listen for these events to know when to resubmit pending Ops.

Hosting application can use these events in order to indicate to user when user changes are not propagating through the system, and thus can be lost (on browser tab being closed). It's advised to use some delay (like 5 seconds) before showing such UI, as network connectivity might be intermittent. Also if container was offline for very long period of time due to `Container.setAutoReconnect(false)` being called, it might take a while to get connected and current.
Hosting application can use these events in order to indicate to user when user changes are not propagating through the system, and thus can be lost (on browser tab being closed). It's advised to use some delay (like 5 seconds) before showing such UI, as network connectivity might be intermittent. Also if container was offline for very long period of time due to `Container.disconnect()` being called, it might take a while to get connected and current.

Please note that hosts can implement various strategies on how to handle disconnections. Some may decide to show some UX letting user know about potential loss of data if container is closed while disconnected. Others can force container to disallow user edits while offline (see [Readonly states](#Readonly-states)).

Expand Down
52 changes: 7 additions & 45 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export enum ConnectionState {
* Useful when resolving URIs and hitting 404, due to container being loaded from (stale) snapshot and not being
* up to date. Host may chose to wait in such case and retry resolving URI.
* Warning: Will wait infinitely for connection to establish if there is no connection.
* May result in deadlock if Container.setAutoReconnect(false) is called and never switched back to auto-reconnect.
* May result in deadlock if Container.disconnect() is called and never followed by a call to Container.connect().
* @returns true: container is up to date, it processed all the ops that were know at the time of first connection
* false: storage does not provide indication of how far the client is. Container processed
* all the ops known to it, but it maybe still behind.
Expand Down Expand Up @@ -231,9 +231,7 @@ export async function waitContainerToCatchUp(container: IContainer) {
};
container.on(connectedEventName, callback);

// TODO: Remove null check after next release #8523
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
container.resume!();
container.connect();
});
}

Expand Down Expand Up @@ -943,30 +941,6 @@ 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;

Expand Down Expand Up @@ -1030,23 +1004,8 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i
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().
// If there is gap, we will learn about it once connected, but the gap should be small (if any),
// assuming that resume() is called quickly after initial container boot.
this.resumeInternal({ reason: "DocumentOpenResume", fetchOpsFromStorage: false });
}
}

private resumeInternal(args: IConnectionArgs) {
assert(!this.closed, 0x0d9 /* "Attempting to setAutoReconnect() a closed DeltaManager" */);
assert(!this.closed, 0x0d9 /* "Attempting to connect() a closed DeltaManager" */);

// Resume processing ops
if (!this.resumedOpProcessingAfterLoad) {
Expand Down Expand Up @@ -1249,7 +1208,10 @@ export class Container extends EventEmitterWithErrorHandling<IContainerEvents> i

switch (loadMode.deltaConnection) {
case undefined:
this.resume();
// 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 resumeInternal() is called quickly after initial container boot.
this.resumeInternal({ reason: "DocumentLoad", fetchOpsFromStorage: false });
break;
case "delayed":
this.resumedOpProcessingAfterLoad = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describeNoCompat("Loader.request", (getTestObjectProvider) => {
}
assert(success, "Loader pause flags doesn't pause container op processing");

(container2 as Container).resume();
(container2 as Container).connect();

// Flush all the ops
await provider.ensureSynchronized();
Expand All @@ -238,7 +238,7 @@ describeNoCompat("Loader.request", (getTestObjectProvider) => {
// this binds newDataStore to dataStore1
dataStore1._root.set("key", newDataStore.handle);

(container1 as Container).resume();
(container1 as Container).connect();

// Flush all the ops
await provider.ensureSynchronized();
Expand Down
Loading

0 comments on commit 3c923a5

Please sign in to comment.