[core-lro] Proposed LRO Engine design

sdk/core/core-lro/docs/LROEngine.md

@@ -0,0 +1,214 @@
+# Modular Support for Long-Running Operations
+
+Long-running operations (LROs) are operations that the service _could_ take a long time to finish processing and they follow a common convention:
+
+- the customer first send an initiation request to the service, which in turn sends back a response, from which the customer can learn how to poll for the status of the operation, if it has not been completed already,
+- using their learnings, the customer polls the status of the operation until it is done,
+- again, using their learnings, the customer can now get the desired result of the operation once its status says it is done.
+
+Ideally, we can write an algorithm that implements this convention once and use it in all Azure clients for LRO APIs, however, in reality, this convention is implemented differently across Azure services. The good news is that the TypeScript Autorest extension is AFAIK able to generate code that implements those different ones, but this implementation has a couple of limitations:
+
+1. it is located in a few files that the code generator copies into every generated package that has LROs. So if in the future there is a bug fix needed in the LRO logic, the package has to be regenerated with the fix.
+2. it works only with clients that use `@azure/core-client`, so clients that use `@azure-rest/core-client` or `@azure/core-http` can not use this implementation as-is.
+
+To fix limitation #1, the most straightforward thing to do is to move those files into `@azure/core-lro`, but without fixing limitation #2 first, `@azure/core-lro` will have to depend on `@azure/core-client` in this case which will force clients that depend on `@azure/core-lro` but not necessarily depend on `@azure/core-client` to transitively depend on the latter, posing concerns about unnecessarily larger bundle sizes.
+
+This document presents a design that fixes limitation #2 and naturally fixes limitation #1 too.
+
+## Things to know before reading
+
+- Some details not related to the high-level concept are not illustrated; the scope of this is limited to the high-level shape and paradigms for the feature area.
+
+## Terminology
+
+- **Azure Async Operation**, **Body**, and **Location** are names for the LRO implementations currently supported in the TypeScript Autorest extension. They vary in how to calculate the path to poll from, the algorithm for detecting whether the operation has finished, and the location to retrieve the desired results from. Currently, these pieces of information can be calculated from the response received after sending the initiation request.
+
+## Why this is needed
+
+The China team is currently waiting for fixing limitation #1 which they regard as a blocker for GAing the TypeScript Autorest extension. Furthermore, having this LRO implementation being part of `@azure/core-lro` and not tied to `@azure/core-client` will significantly help streamline the underway effort to add convenience helpers for LROs in `@azure-rest` clients.
+
+## Proposed design
+
+This document presents a design of an LRO engine to be part of `@azure/core-lro` and could be used by any client regardless of how it is implemented. Furthermore, specific implementations of the engine are also provided to be auto-generated by Autorest.
+
+The design consists of three main pieces:
+
+- an interface, named `LongRunningOperation<T>` which groups various primitives needed to implement LROs
+- a class, named `LroEngine`, that implements the LRO engine and its constructor takes as input an object that implements `LongRunningOperation<T>`
+- classes that implement `LongRunningOperation<T>` for `@azure/core-http` and `@azure/core-client`. @joheredi also created one for `@azure-rest/core-client` in https://github.com/Azure/azure-sdk-for-js/pull/15898
+
+### `LongRunningOperation<T>`
+
+This interface contains the following three methods: **sendInitialRequest**, **sendPollRequest**, and **retrieveAzureAsyncResource**. I propose to make this interface exported by `@azure/core-lro`.
+
+#### `sendInitialRequest`
+
+This method should be implemented to send the initial request to start the operation and it has the following signature:
+
+```ts
+sendInitialRequest: (initializeState: (rawResponse: RawResponse, flatResponse: unknown) => boolean) => Promise<LroResponse<T>>
+```
+
+The `initializeState` parameter is a function that will initialize the state of the polling operation and will return a Boolean that indicates whether the operation has already been completed. This Boolean is especially useful in `@azure/core-client`'s engine because at this point, the customer-provided callback should be called. The implementation for `@azure/core-http` looks like this:
+
+```ts
+public async sendInitialRequest(
+  initializeState: (
+    rawResponse: RawResponse,
+    flatResponse: unknown
+  ) => boolean
+): Promise<LroResponse<T>> {
+  const response = await this.sendOperation(this.args, this.spec); // the class will have sendOperation, args, and spec as private fields
+  initializeState(response.rawResponse, response.flatResponse);
+  return response;
+}
+```
+
+#### `sendPollRequest`
+
+This method should be implemented to send a polling request, a request the service should respond to with the status of the operation, and it has the following signature:
+
+```ts
+sendPollRequest: (config: LroConfig, path: string) => Promise<LroStatus<T>>
+```
+
+This method takes two things as input, it takes the request path as you would expect but also takes a parameter of type `LroConfig`. The latter is an object that holds meta information about how the LRO should work and clients could need access to this information to do things like doing custom de-serialization in certain cases where the swagger does not describe all response shapes. The `LroConfig` interface is defined as follows:
+
+```ts
+export interface LroConfig {
+  mode?: LroMode;
+  resourceLocation?: string;
+}
+```
+
+The information in it is currently populated from the response of the initial request. `mode` specifies which LRO implementation this particular API follows (see the [Terminology](#terminology) section). Furthermore, `resourceLocation` is a path where the desired result of the LRO could potentially be located.
+
+Implementing `sendPollRequest` is tricky because it returns a `LroStatus` which has a `done` field and it is not easy to figure out whether the LRO has finished, so this design exposes a helper function, called `createGetLroStatusFromResponse`, which the implementation should call to get a `getLroStatusFromResponse`, which in turn should be called on the response to detect whether the operation has finished at this point. Here is what the implementation looks like for `@azure/core-http`, ignoring the custom de-serialization stuff:
+
+```ts
+public async sendPollRequest(
+    config: LroConfig,
+    path: string
+): Promise<LroStatus<T>> {
+  const getLroStatusFromResponse = createGetLroStatusFromResponse(this, config, this.finalStateVia);
+  const { flatResponse, rawResponse } = await sendOperation(this.args, {
+    ...this.spec,
+    httpMethod: "GET",
+    path: path)
+  });
+  return getLroStatusFromResponse(rawResponse, flatResponse as T);
+}
+```
+
+#### `retrieveAzureAsyncResource`
+
+This method is only needed to potentially retrieve the provisioned Azure resource in the _Azure Async Operation_ mode after the operation has finished and it should be different from the `sendPollRequest` one in two things:
+
+- should set the obscure `shouldDeserialize` option needed for doing custom de-serialization on the result if it is supported by the client
+- we already know the operation has finished at this point, so it just returns an object of type `LroResponse` instead of `LroStatus`.
+
+It has the following signature:
+
+```ts
+retrieveAzureAsyncResource: (path?: string) => Promise<LroResponse<T>>
+```
+
+Note that `path` is optional here because `resourceLocation` is also optional. Here is the implementation for `@azure/core-http`:
+
+```ts
+  public async retrieveAzureAsyncResource(
+    path?: string
+  ): Promise<LroStatus<T>> {
+  const updatedArgs = { ...this.args };
+  if (updatedArgs.options) {
+    (updatedArgs.options as any).shouldDeserialize = true;
+  }
+    return sendOperation(updatedArgs, {
+      ...this.spec,
+      httpMethod: "GET",
+      ...(path && { path })
+    });
+  }
+```
+
+### `LroEngine`
+
+I propose to make this class also exported by `@azure/core-lro`. This class has the following type signature:
+
+```ts
+class LroEngine<TResult, TState extends PollOperationState<TResult>> extends Poller<TState, TResult>
+```
+
+so a client can instantiate the state of the polling operation with a custom type that extends the standard `PollOperationState`. This flexibility can open the door for this class to be used in Track 2 packages where it is common for LROs to have other interesting information as part of their state and customers might want to have access to it, e.g. the last time the operation was updated.
+
+The class also has the following constructor:
+
+```ts
+constructor(lro: LongRunningOperation<TResult>, options?: LroEngineOptions);
+```
+
+Currently `options` have `intervalInMs` to control the polling interval and `resumeFrom` to enable resuming from a serialized state. If there are new arguments to be added to the class, they could be added to the options type. For instance, we might decide to enable customers to provide a custom LRO implementation in the form of a custom [`update`](https://github.com/Azure/azure-sdk-for-js/blob/2d2c6561cac330b8720763db88705fad4e867bda/sdk/core/core-lro/src/pollOperation.ts#L63) method, such a method could be part of the options.
+
+### [`coreClientLro`](https://github.com/Azure/autorest.typescript/blob/main/src/coreClientLro.ts) and [`coreHttpLro`](https://github.com/Azure/autorest.typescript/blob/main/src/coreHttpLro.ts)
+
+I propose to make these classes auto-generated by Autorest. These classes implement `LongRunningOperation<T>` and various bits of `coreHttpLro` have been shown earlier. These classes will need access to a few pieces: operation specification and operation arguments and a primitive function that can take them as input to send a request and also converts the response into one of type `LroResponse<T>` which has both the flattened and the raw responses. Furthermore, those classes can also, optionally, take as input an object of type `LroResourceLocationConfig`, which could determine where to find the results of the LRO after the operation is finished. Typically, Autorest figures out the value for `LroResourceLocationConfig` from the `x-ms-long-running-operation-options` swagger extension.
+
+## Usage examples
+
+### Create an object of `coreClientLro`
+
+```ts
+const directSendOperation = async (
+  args: coreClient.OperationArguments,
+  spec: coreClient.OperationSpec
+): Promise<unknown> => {
+  return this.client.sendOperationRequest(args, spec);
+};
+const sendOperation = async (
+  args: coreClient.OperationArguments,
+  spec: coreClient.OperationSpec
+) => {
+  let currentRawResponse: coreClient.FullOperationResponse | undefined = undefined;
+  const providedCallback = args.options?.onResponse;
+  const callback: coreClient.RawResponseCallback = (
+    rawResponse: coreClient.FullOperationResponse,
+    flatResponse: unknown
+  ) => {
+    currentRawResponse = rawResponse;
+    providedCallback?.(rawResponse, flatResponse);
+  };
+  const updatedArgs = {
+    ...args,
+    options: {
+      ...args.options,
+      onResponse: callback
+    }
+  };
+  const flatResponse = await directSendOperation(updatedArgs, spec);
+  return {
+    flatResponse,
+    rawResponse: {
+      statusCode: currentRawResponse!.status,
+      body: currentRawResponse!.parsedBody,
+      headers: currentRawResponse!.headers.toJSON()
+    }
+  };
+};
+
+const lro = new CoreClientLro(
+  sendOperation,
+  { options }, // arguments are just the operation options
+  spec
+);
+```
+
+### Using `LroEngine`
+
+```ts
+const pollerEngine = new LroEngine(lro, { intervalInMs: 2000 }); // lro was instantiated in the previous section
+const result = pollerEngine.pollUntilDone();
+```
+
+## Testing
+
+We have [extensive test suite for LROs](https://github.com/Azure/autorest.typescript/blob/main/test/integration/lro.spec.ts) in the TypeScript code generator repo. I both added those tests here and re-implemented the [lro routes](https://github.com/Azure/autorest.testserver/blob/main/legacy/routes/lros.js) in the Autorest test server. For this to work, I created a [fairly low-level instantiation for `LongRunningOperation<T>` with just `@azure/core-rest-pipeline`](https://github.com/deyaaeldeen/azure-sdk-for-js/blob/lro-design/sdk/core/core-lro/test/utils/coreRestPipelineLro.ts).

sdk/core/core-lro/package.json

@@ -65,14 +65,11 @@
   },
   "scripts": {
     "audit": "node ../../../common/scripts/rush-audit.js && rimraf node_modules package-lock.json && npm i --package-lock-only 2>&1 && npm audit",
-    "build": "tsc -p . && npm run build:nodebrowser && api-extractor run --local",
+    "build": "tsc -p . && rollup -c 2>&1 && api-extractor run --local",
     "build:samples": "echo Skipped.",
-    "build:nodebrowser": "rollup -c 2>&1",
     "build:node": "tsc -p . && cross-env ONLY_NODE=true rollup -c 2>&1 && npm run extract-api",
     "build:browser": "tsc -p . && cross-env ONLY_BROWSER=true rollup -c 2>&1",
-    "build:test": "tsc -p . && rollup -c rollup.test.config.js 2>&1",
-    "build:test:node": "tsc -p . && cross-env ONLY_NODE=true rollup -c rollup.test.config.js 2>&1",
-    "build:test:browser": "tsc -p . && cross-env ONLY_BROWSER=true rollup -c rollup.test.config.js 2>&1",
+    "build:test": "tsc -p . && rollup -c 2>&1",
     "check-format": "prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"*.{js,json}\" \"test/**/*.ts\" \"samples/**/*.ts\"",
     "clean": "rimraf dist dist-* types *.log browser statistics.html coverage src/**/*.js test/**/*.js",
     "execute:samples": "echo skipped",
@@ -88,7 +85,7 @@
     "test": "npm run build:test && npm run unit-test",
     "unit-test": "npm run unit-test:node && npm run unit-test:browser",
     "unit-test:browser": "karma start --single-run",
-    "unit-test:node": "nyc mocha --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"dist-test/index.node.js\"",
+    "unit-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"dist-esm/test/{,!(browser)/**/}*.spec.js\"",
     "pack": "npm pack 2>&1",
     "prebuild": "npm run clean",
     "docs": "typedoc --excludePrivate --excludeNotExported --excludeExternals --stripInternal --mode file --out ./dist/docs ./src"
@@ -102,13 +99,10 @@
   },
   "devDependencies": {
     "@azure/core-http": "^2.0.0",
+    "@azure/core-rest-pipeline": "^1.1.0",
     "@azure/eslint-plugin-azure-sdk": "^3.0.0",
     "@azure/dev-tool": "^1.0.0",
     "@microsoft/api-extractor": "7.7.11",
-    "@rollup/plugin-commonjs": "11.0.2",
-    "@rollup/plugin-multi-entry": "^3.0.0",
-    "@rollup/plugin-node-resolve": "^8.0.0",
-    "@rollup/plugin-replace": "^2.2.0",
     "@types/chai": "^4.1.6",
     "@types/mocha": "^7.0.2",
     "@types/node": "^12.0.0",
@@ -134,10 +128,6 @@
     "prettier": "^1.16.4",
     "rimraf": "^3.0.0",
     "rollup": "^1.16.3",
-    "rollup-plugin-shim": "^1.0.0",
-    "rollup-plugin-sourcemaps": "^0.4.2",
-    "rollup-plugin-terser": "^5.1.1",
-    "rollup-plugin-visualizer": "^4.0.4",
     "ts-node": "^9.0.0",
     "typescript": "~4.2.0",
     "uglify-js": "^3.4.9",

sdk/core/core-lro/review/core-lro.api.md

@@ -9,6 +9,65 @@ import { AbortSignalLike } from '@azure/abort-controller';
 // @public
 export type CancelOnProgress = () => void;
 
+// @public
+export function createGetLroStatusFromResponse<TResult>(lroPrimitives: LongRunningOperation<TResult>, config: LroConfig, lroResourceLocationConfig?: LroResourceLocationConfig): GetLroStatusFromResponse<TResult>;
+
+// @public
+export type GetLroStatusFromResponse<T> = (rawResponse: RawResponse, flatResponse: T) => LroStatus<T>;
+
+// @public
+export interface LongRunningOperation<T> {
+    requestMethod: string;
+    requestPath: string;
+    retrieveAzureAsyncResource: (path?: string) => Promise<LroResponse<T>>;
+    sendInitialRequest: (initializeState: (rawResponse: RawResponse, flatResponse: unknown) => boolean) => Promise<LroResponse<T>>;
+    sendPollRequest: (config: LroConfig, path: string) => Promise<LroStatus<T>>;
+}
+
+// @public
+export interface LroConfig {
+    mode?: LroMode;
+    resourceLocation?: string;
+}
+
+// @public
+export class LroEngine<TResult, TState extends PollOperationState<TResult>> extends Poller<TState, TResult> {
+    constructor(lro: LongRunningOperation<TResult>, options?: LroEngineOptions);
+    delay(): Promise<void>;
+    }
+
+// @public
+export interface LroEngineOptions {
+    intervalInMs?: number;
+    resumeFrom?: string;
+}
+
+// @public
+export interface LroInProgressState<T> extends LroResponse<T> {
+    done: false;
+    next?: () => Promise<LroStatus<T>>;
+}
+
+// @public
+export type LroMode = "AzureAsync" | "Location" | "Body";
+
+// @public
+export type LroResourceLocationConfig = "azure-async-operation" | "location" | "original-uri";
+
+// @public
+export interface LroResponse<T> {
+    flatResponse: T;
+    rawResponse: RawResponse;
+}
+
+// @public
+export type LroStatus<T> = LroTerminalState<T> | LroInProgressState<T>;
+
+// @public
+export interface LroTerminalState<T> extends LroResponse<T> {
+    done: true;
+}
+
 // @public
 export abstract class Poller<TState extends PollOperationState<TResult>, TResult> implements PollerLike<TState, TResult> {
     constructor(operation: PollOperation<TState, TResult>);
@@ -83,6 +142,15 @@ export interface PollOperationState<TResult> {
 // @public
 export type PollProgressCallback<TState> = (state: TState) => void;
 
+// @public
+export interface RawResponse {
+    body?: unknown;
+    headers: {
+        [headerName: string]: string;
+    };
+    statusCode: number;
+}
+
 
 // (No @packageDocumentation comment for this package)