Skip to content

Commit

Permalink
Split public and internal IFluidHandle APIs (#20123)
Browse files Browse the repository at this point in the history
## Description

Split IFluidHandle into two interfaces, IFluidHandle and
IFluidHandleInternal.

## Breaking Changes

Deprecated members of IFluidHandle are split off into new
IFluidHandleInternal interface.

See included changeset for more details.
  • Loading branch information
CraigMacomber committed May 3, 2024
1 parent 0a7921d commit ed0bbe4
Show file tree
Hide file tree
Showing 106 changed files with 904 additions and 464 deletions.
25 changes: 25 additions & 0 deletions .changeset/clever-breads-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
"@fluidframework/core-interfaces": minor
"fluid-framework": minor
"@fluidframework/map": minor
"@fluidframework/matrix": minor
"@fluidframework/ordered-collection": minor
"@fluidframework/register-collection": minor
"@fluidframework/runtime-definitions": minor
"@fluidframework/runtime-utils": minor
"@fluidframework/sequence": minor
"@fluid-experimental/sequence-deprecated": minor
"@fluidframework/shared-object-base": minor
"@fluidframework/synthesize": minor
"@fluid-experimental/tree": minor
"@fluidframework/tree": minor
---

Deprecated members of IFluidHandle are split off into new IFluidHandleInternal interface

Split IFluidHandle into two interfaces, `IFluidHandle` and `IFluidHandleInternal`.
Code depending on the previously deprecated members of IFluidHandle can access them by using `toFluidHandleInternal` from `@fluidframework/runtime-utils/legacy`.

External implementation of the `IFluidHandle` interface are not supported: this change makes the typing better convey this using the `ErasedType` pattern.
Any existing and previously working, and now broken, external implementations of `IFluidHandle` should still work at runtime, but will need some unsafe type casts to compile.
Such handle implementation may break in the future and thus should be replaced with use of handles produced by the Fluid Framework client packages.
6 changes: 3 additions & 3 deletions experimental/dds/attributable-map/src/test/mocha/map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { strict as assert } from "assert";

import { AttachState } from "@fluidframework/container-definitions";
import { IFluidHandle } from "@fluidframework/core-interfaces";
import type { IFluidHandleInternal } from "@fluidframework/core-interfaces/internal";
import { ISummaryBlob } from "@fluidframework/protocol-definitions";
import {
MockContainerRuntimeFactory,
Expand Down Expand Up @@ -588,7 +588,7 @@ describe("Map", () => {
containerRuntimeFactory.processAllMessages();

// Verify the local SharedMap
const localSubMap = map1.get<IFluidHandle>("test");
const localSubMap = map1.get<IFluidHandleInternal>("test");
assert(localSubMap);
assert.equal(
localSubMap.absolutePath,
Expand All @@ -597,7 +597,7 @@ describe("Map", () => {
);

// Verify the remote SharedMap
const remoteSubMap = map2.get<IFluidHandle>("test");
const remoteSubMap = map2.get<IFluidHandleInternal>("test");
assert(remoteSubMap);
assert.equal(
remoteSubMap.absolutePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { strict as assert } from "assert";

import { IGCTestProvider, runGCTests } from "@fluid-private/test-dds-utils";
import { IFluidHandle } from "@fluidframework/core-interfaces";
import type { IFluidHandleInternal } from "@fluidframework/core-interfaces/internal";
import {
MockContainerRuntimeFactory,
MockFluidDataStoreRuntime,
Expand Down Expand Up @@ -35,7 +35,7 @@ function createConnectedSequence(id: string, runtimeFactory: MockContainerRuntim

function createLocalSequence(id: string) {
const factory = new SharedObjectSequenceFactory();
return factory.create(new MockFluidDataStoreRuntime(), id);
return factory.create(new MockFluidDataStoreRuntime(), id) as SharedObjectSequence<unknown>;
}

describe("SharedObjectSequence", () => {
Expand Down Expand Up @@ -80,7 +80,9 @@ describe("SharedObjectSequence", () => {
assert(this.sequence1.getLength() > 0, "Route must be added before deleting");
const lastElementIndex = this.sequence1.getLength() - 1;
// Get the handles that were last added.
const deletedHandles = this.sequence1.getRange(lastElementIndex) as IFluidHandle[];
const deletedHandles = this.sequence1.getRange(
lastElementIndex,
) as IFluidHandleInternal[];
// Get the routes of the handles.
const deletedHandleRoutes = Array.from(
deletedHandles,
Expand Down
4 changes: 2 additions & 2 deletions experimental/dds/tree/src/PayloadUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { isFluidHandle } from '@fluidframework/runtime-utils/internal';
import { isFluidHandle, toFluidHandleInternal } from '@fluidframework/runtime-utils/internal';
import { compareArrays } from '@fluidframework/core-utils/internal';

import { Payload } from './persisted-types/index.js';
Expand Down Expand Up @@ -67,7 +67,7 @@ export function comparePayloads(a: Payload, b: Payload): boolean {
// Special case IFluidHandles, comparing them only by their absolutePath
if (isFluidHandle(a)) {
if (isFluidHandle(b)) {
return a.absolutePath === b.absolutePath;
return toFluidHandleInternal(a).absolutePath === toFluidHandleInternal(b).absolutePath;
}
return false;
}
Expand Down
20 changes: 10 additions & 10 deletions experimental/dds/tree/src/migration-shim/shimHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* Licensed under the MIT License.
*/

import { type IFluidHandle } from '@fluidframework/core-interfaces';
import { type IFluidHandleInternal } from '@fluidframework/core-interfaces/internal';
import { FluidHandleBase, toFluidHandleInternal } from '@fluidframework/runtime-utils/internal';

import { type IShim } from './types.js';

Expand All @@ -15,25 +16,24 @@ import { type IShim } from './types.js';
* Local handles such as the FluidObjectHandle and the SharedObjectHandle don't work as they do not properly bind the
* Shim's underlying DDS.
*/
export class ShimHandle<TShim extends IShim> implements IFluidHandle<TShim> {
public constructor(private readonly shim: TShim) {}
export class ShimHandle<TShim extends IShim> extends FluidHandleBase<TShim> {
public constructor(private readonly shim: TShim) {
super();
}

public get absolutePath(): string {
return this.shim.currentTree.handle.absolutePath;
return toFluidHandleInternal(this.shim.currentTree.handle).absolutePath;
}
public get isAttached(): boolean {
return this.shim.currentTree.handle.isAttached;
}
public attachGraph(): void {
return this.shim.currentTree.handle.attachGraph();
return toFluidHandleInternal(this.shim.currentTree.handle).attachGraph();
}
public async get(): Promise<TShim> {
return this.shim;
}
public bind(handle: IFluidHandle): void {
return this.shim.currentTree.handle.bind(handle);
}
public get IFluidHandle(): IFluidHandle<TShim> {
return this;
public bind(handle: IFluidHandleInternal): void {
return toFluidHandleInternal(this.shim.currentTree.handle).bind(handle);
}
}
19 changes: 15 additions & 4 deletions experimental/dds/tree/src/test/EditUtilities.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
* Licensed under the MIT License.
*/

import { IFluidHandle } from '@fluidframework/core-interfaces';
import { IFluidHandle, fluidHandleSymbol } from '@fluidframework/core-interfaces';
import { assert } from '@fluidframework/core-utils/internal';
import { FluidSerializer } from '@fluidframework/shared-object-base/internal';
import { MockFluidDataStoreRuntime } from '@fluidframework/test-runtime-utils/internal';
import { expect } from 'chai';
import type { IFluidHandleInternal } from '@fluidframework/core-interfaces/internal';

import { BuildNode, BuildTreeNode } from '../ChangeTypes.js';
import { noop } from '../Common.js';
Expand Down Expand Up @@ -454,7 +455,12 @@ describe('EditUtilities', () => {
new MockFluidDataStoreRuntime().IFluidHandleContext,
() => {}
);
const binder: IFluidHandle = { bind: noop } as unknown as IFluidHandle;
const binder: IFluidHandle = {
bind: noop,
get [fluidHandleSymbol]() {
return binder;
},
} as unknown as IFluidHandle;

enum Equality {
Equal,
Expand Down Expand Up @@ -614,9 +620,14 @@ describe('EditUtilities', () => {
// This is used instead of MockHandle so equal handles compare deeply equal.
function makeMockHandle(data: string): IFluidHandle {
// `/` prefix is needed to prevent serializing from modifying handle.
const handleObject = { absolutePath: `/${data}`, IFluidHandle: undefined as unknown };
const handleObject = {
absolutePath: `/${data}`,
IFluidHandle: undefined as unknown,
[fluidHandleSymbol]: undefined as any,
};
handleObject.IFluidHandle = handleObject;
return handleObject as IFluidHandle;
handleObject[fluidHandleSymbol] = handleObject;
return handleObject as unknown as IFluidHandleInternal;
}
// Theoretically handles serialize as objects with 2 fields and thus serialization is allowed to be non-deterministic
// so use allEqualUnstable not allEqual.
Expand Down
9 changes: 3 additions & 6 deletions experimental/dds/tree/src/test/utilities/TestSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* Licensed under the MIT License.
*/

import { IFluidHandle, IRequest, IResponse } from '@fluidframework/core-interfaces';
import { IFluidHandle, IRequest, IResponse } from '@fluidframework/core-interfaces/internal';
import { FluidHandleBase } from '@fluidframework/runtime-utils/internal';
import { IFluidSerializer } from '@fluidframework/shared-object-base';

export class TestFluidSerializer implements IFluidSerializer {
Expand All @@ -30,14 +31,10 @@ export class TestFluidSerializer implements IFluidSerializer {
}
}

export class TestFluidHandle implements IFluidHandle {
export class TestFluidHandle extends FluidHandleBase<unknown> {
public absolutePath;
public isAttached;

public get IFluidHandle(): IFluidHandle {
return this;
}

public async get(): Promise<any> {
throw new Error('Method not implemented.');
}
Expand Down
29 changes: 19 additions & 10 deletions packages/common/core-interfaces/api-report/core-interfaces.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ export const FluidErrorTypes: {
// @alpha (undocumented)
export type FluidErrorTypes = (typeof FluidErrorTypes)[keyof typeof FluidErrorTypes];

// @public
export const fluidHandleSymbol: unique symbol;

// @public
export type FluidObject<T = unknown> = {
[P in FluidObjectProviderKeys<T>]?: T[P];
Expand Down Expand Up @@ -240,13 +243,8 @@ export type IEventTransformer<TThis, TEvent extends IEvent> = TEvent extends {
export const IFluidHandle = "IFluidHandle";

// @public
export interface IFluidHandle<out T = FluidObject & IFluidLoadable> extends IProvideFluidHandle {
// @deprecated (undocumented)
readonly absolutePath: string;
// @deprecated (undocumented)
attachGraph(): void;
// @deprecated (undocumented)
bind(handle: IFluidHandle): void;
export interface IFluidHandle<out T = unknown> {
readonly [fluidHandleSymbol]: IFluidHandleErased<T>;
get(): Promise<T>;
readonly isAttached: boolean;
}
Expand All @@ -264,6 +262,17 @@ export interface IFluidHandleContext extends IProvideFluidHandleContext {
readonly routeContext?: IFluidHandleContext;
}

// @public
export interface IFluidHandleErased<T> extends ErasedType<readonly ["IFluidHandle", T]> {
}

// @alpha
export interface IFluidHandleInternal<out T = unknown> extends IFluidHandle<T>, IProvideFluidHandle {
readonly absolutePath: string;
attachGraph(): void;
bind(handle: IFluidHandleInternal): void;
}

// @public (undocumented)
export const IFluidLoadable: keyof IProvideFluidLoadable;

Expand Down Expand Up @@ -296,10 +305,10 @@ export interface ILoggingError extends Error {
getTelemetryProperties(): ITelemetryBaseProperties;
}

// @public (undocumented)
// @alpha @deprecated (undocumented)
export interface IProvideFluidHandle {
// (undocumented)
readonly [IFluidHandle]: IFluidHandle;
// @deprecated (undocumented)
readonly [IFluidHandle]: IFluidHandleInternal;
}

// @public (undocumented)
Expand Down
22 changes: 21 additions & 1 deletion packages/common/core-interfaces/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,26 @@
"typescript": "~5.1.6"
},
"typeValidation": {
"broken": {}
"broken": {
"RemovedVariableDeclaration_isFluidPackage": {
"backCompat": false,
"forwardCompat": false
},
"InterfaceDeclaration_IFluidHandle": {
"forwardCompat": false,
"backCompat": false
},
"InterfaceDeclaration_IProvideFluidHandle": {
"forwardCompat": false
},
"InterfaceDeclaration_IProvideFluidLoadable": {
"forwardCompat": false,
"backCompat": false
},
"InterfaceDeclaration_IFluidLoadable": {
"forwardCompat": false,
"backCompat": false
}
}
}
}
Loading

0 comments on commit ed0bbe4

Please sign in to comment.