Repository: neet/masto.js
Pull request #81 (2019-06-28 06:24:41)

Fix behaviour of pagination generator

codecov-io: # [Codecov](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=h1) Report
> Merging [#81](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=desc) into [master](https://codecov.io/gh/neet/masto.js/commit/d3c95a549abd65407a4dce25a5a938b15ae446ae?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/neet/masto.js/pull/81/graphs/tree.svg?width=650&token=SW9FhwMnZ3&height=150&src=pr)](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master    #81   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     10           
  Lines         436    429    -7     
  Branches       38     39    +1     
=====================================
- Hits          436    429    -7
```


| [Impacted Files](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/gateway/gateway.ts](https://codecov.io/gh/neet/masto.js/pull/81/diff?src=pr&el=tree#diff-c3JjL2dhdGV3YXkvZ2F0ZXdheS50cw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=footer). Last update [d3c95a5...192d4b7](https://codecov.io/gh/neet/masto.js/pull/81?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/gateway/gateway.ts b/src/gateway/gateway.ts
index 40429957..0a74c8dd 100644
--- a/src/gateway/gateway.ts
+++ b/src/gateway/gateway.ts
@@ -1,9 +1,9 @@
-import axios, { AxiosInstance, AxiosRequestConfig } from 'axios';
+import axios, { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios';
 import normalizeUrl from 'normalize-url';
 import querystring from 'querystring';
 import semver from 'semver';
 import { Omit } from 'simplytyped';
-import * as oc from 'ts-optchain';
+import * as optchain from 'ts-optchain';
 import { Instance } from '../entities/instance';
 import { MastoNotFoundError } from '../errors/masto-not-found-error';
 import { MastoRateLimitError } from '../errors/masto-rate-limit-error';
@@ -12,6 +12,8 @@ import { createFormData } from './create-form-data';
 import { isAxiosError } from './is-axios-error';
 import { WebSocketEvents } from './websocket';
 
+const { oc } = optchain;
+
 export interface GatewayConstructorParams {
   /** URI of the instance */
   uri: string;
@@ -31,14 +33,17 @@ export type LoginParams = Omit<
   'version' | 'streamingApiUrl'
 >;
 
-export type PaginateNextOptions<Params> = {
-  /** Reset pagination */
-  reset?: boolean;
-  /** URL */
-  url?: string;
-  /** Query parameters */
-  params?: Params;
-};
+export type PaginateNextOptions =
+  | {
+      reset: boolean;
+      url?: undefined;
+      params?: undefined;
+    }
+  | {
+      reset?: undefined;
+      url: string;
+      params?: any;
+    };
 
 /**
  * Mastodon network request wrapper
@@ -179,11 +184,11 @@ export class Gateway {
         throw error;
       }
 
-      const status = oc.oc(error).response.status();
+      const status = oc(error).response.status();
 
       // Error response from REST API might contain error key
       // https://docs.joinmastodon.org/api/entities/#error
-      const { error: errorMessage } = oc.oc(error).response.data({
+      const { error: errorMessage } = oc(error).response.data({
         error: 'Unexpected error',
       });
 
@@ -339,59 +344,38 @@ export class Gateway {
 
   /**
    * Generate an iterable of the pagination.
-   * The default generator implementation of JS cannot change the value of `done` depend on the result of yield,
-   * Therefore we define custom generator to reproduce Mastodon's link header behaviour faithfully.
-   * @param path Path for the endpoint
+   * @param initialURL Path for the endpoint
    * @param initialParams Query parameter
    * @return Async iterable iterator of the pages.
    * See also [MDN article about generator/iterator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Iterators_and_Generators)
    */
-  public paginate<Data, Params = any>(
-    path: string,
-    initialParams?: Params,
-  ): AsyncIterableIterator<Data | undefined> {
-    // tslint:disable-next-line no-this-assignment
-    const gateway = this;
-    const initialUrl = this.uri + path;
-
-    let currentUrl: string = initialUrl;
-    let currentParams: Params | undefined = initialParams;
-
-    return {
-      async next(options: PaginateNextOptions<Params> = {}) {
-        if (options.reset) {
-          currentUrl = initialUrl;
-          currentParams = initialParams;
-        }
-
-        const response = await gateway.request<Data>({
-          method: 'GET',
-          url: options.url || currentUrl,
-          params: options.params || currentParams,
-        });
-
-        // Set next url from the link header
-        const link = oc.oc(response.headers).link('');
-        const match = link.match(/<(.+?)>; rel="next"/) as string[];
-
-        currentUrl = (match && match.length && match[1]) || '';
-        currentParams = undefined;
+  public async *paginate<Data>(initialUrl: string, initialParams?: any) {
+    let nextUrl: string | undefined = initialUrl;
+    let nextParams = initialParams;
+
+    while (nextUrl) {
+      const response: AxiosResponse<Data> = await this.request<Data>({
+        method: 'GET',
+        url: nextUrl,
+        params: nextParams,
+      });
 
-        // Return `done: true` immediately if no next url returned
-        return { done: !currentUrl, value: response.data };
-      },
+      // Yield can be argument of next()
+      const options: PaginateNextOptions = yield response.data;
 
-      async return(value: Data) {
-        return { value, done: true };
-      },
+      // Get next URL from "next" in the link header
+      const link = oc(response.headers)
+        .link('')
+        .match(/<(.+?)>; rel="next"/) as string[];
+      const match = link && link.length ? link[1] : undefined;
 
-      async throw(error?: Error) {
-        throw error;
-      },
+      nextUrl = oc(options).url() || match;
+      nextParams = oc(options).params();
 
-      [Symbol.asyncIterator]() {
-        return this;
-      },
-    };
+      if (oc(options).reset()) {
+        nextUrl = initialUrl;
+        nextParams = initialParams;
+      }
+    }
   }
 }


Test patch:
diff --git a/src/gateway/__tests__/gateway.spec.ts b/src/gateway/__tests__/gateway.spec.ts
index 456b7981..dc358e3e 100644
--- a/src/gateway/__tests__/gateway.spec.ts
+++ b/src/gateway/__tests__/gateway.spec.ts
@@ -17,9 +17,7 @@ const mockAxios = jest.genMockFromModule<typeof axios>('axios');
 (axios.create as jest.Mock).mockImplementation(() => mockAxios);
 
 describe('Gateway', () => {
-  class InheritedGateway extends Gateway {}
-
-  const gateway = new InheritedGateway({
+  const gateway = new Gateway({
     uri: 'https://example.com',
     version: '99.9.9',
     streamingApiUrl: 'wss://example.com',
@@ -27,6 +25,8 @@ describe('Gateway', () => {
 
   beforeEach(() => {
     ((axios as any) as jest.Mock).mockClear();
+    ((mockAxios as any) as jest.Mock).mockClear();
+    ((mockAxios.request as any) as jest.Mock).mockClear();
     ((mockAxios.request as any) as jest.Mock).mockResolvedValue({
       data: undefined,
     });
@@ -46,14 +46,14 @@ describe('Gateway', () => {
       uri: 'https://example.com',
       accessToken: 'tokentoken',
     };
-    const gateway = await InheritedGateway.login(params);
+    const gateway = await Gateway.login(params);
 
     expect(gateway.version).toBe('2.8.0');
     expect(gateway.streamingApiUrl).toBe('wss://example.com/stream');
   });
 
   test('streamingApiUrl has been set if construct with streamingApiUrl', () => {
-    const customGateway = new InheritedGateway({
+    const customGateway = new Gateway({
       uri: 'https://example.com',
       streamingApiUrl: 'wss://example.com',
     });
@@ -61,7 +61,7 @@ describe('Gateway', () => {
   });
 
   test('version has been set if construct with version ', () => {
-    const customGateway = new InheritedGateway({
+    const customGateway = new Gateway({
       uri: 'https://example.com',
       version: '1.2.3',
     });
@@ -69,7 +69,7 @@ describe('Gateway', () => {
   });
 
   test('accessToken has been set if construct with accessToken', () => {
-    const customGateway = new InheritedGateway({
+    const customGateway = new Gateway({
       uri: 'https://example.com',
       accessToken: 'token token',
     });
@@ -77,7 +77,7 @@ describe('Gateway', () => {
   });
 
   test('this._uri accessor works', () => {
-    const customGateway = new InheritedGateway({
+    const customGateway = new Gateway({
       uri: 'https://example.com/aaa',
     });
     customGateway.uri = 'https://example.com/bbb';
@@ -85,7 +85,7 @@ describe('Gateway', () => {
   });
 
   test('this._streamingApiUrl accessor works', () => {
-    const customGateway = new InheritedGateway({
+    const customGateway = new Gateway({
       uri: 'wss://example.com/aaa',
     });
     customGateway.uri = 'wss://example.com/bbb';
@@ -332,22 +332,25 @@ describe('Gateway', () => {
 
     const firstResponse = {
       headers: {
-        // Pretend `/next` the next url
-        link: '<https://example.com/next>; rel="next"',
+        // Pretend `/page1` the next url
+        link: '<https://example.com/page1>; rel="next"',
       },
       data: {
         a: 'a',
       },
     };
 
-    ((mockAxios.request as any) as jest.Mock).mockResolvedValue(firstResponse);
+    ((mockAxios.request as any) as jest.Mock).mockResolvedValueOnce(
+      firstResponse,
+    );
 
     const firstResult = await iterable.next();
 
-    expect((mockAxios.request as any) as jest.Mock).toBeCalledWith(
+    expect(mockAxios.request).toBeCalledTimes(1);
+    expect((mockAxios.request as any) as jest.Mock).toHaveBeenLastCalledWith(
       expect.objectContaining({
         method: 'GET',
-        url: 'https://example.com/',
+        url: '/',
         params: {
           since_id: '123',
         },
@@ -358,7 +361,7 @@ describe('Gateway', () => {
 
     const secondResponse = {
       headers: {
-        // Next url is null so `done` should be true
+        // Next url is null
         link: '',
       },
       data: {
@@ -366,25 +369,36 @@ describe('Gateway', () => {
       },
     };
 
-    ((mockAxios.request as any) as jest.Mock).mockResolvedValue(secondResponse);
+    ((mockAxios.request as any) as jest.Mock).mockResolvedValueOnce(
+      secondResponse,
+    );
 
-    const result = await iterable.next();
+    const secondResult = await iterable.next();
 
-    expect((mockAxios.request as any) as jest.Mock).toBeCalledWith(
+    expect(mockAxios.request).toBeCalledTimes(2);
+    expect((mockAxios.request as any) as jest.Mock).toHaveBeenLastCalledWith(
       expect.objectContaining({
         method: 'GET',
-        url: 'https://example.com/next',
+        url: 'https://example.com/page1',
       }),
     );
-    expect(result.done).toBe(true);
-    expect(result.value).toBe(secondResponse.data);
+    expect(secondResult.done).toBe(false);
+    expect(secondResult.value).toBe(secondResponse.data);
+
+    ((mockAxios.request as any) as jest.Mock).mockResolvedValueOnce({
+      data: undefined,
+    });
+    const thirdReuslt = await iterable.next();
+
+    expect(mockAxios.request).toBeCalledTimes(2);
+    expect(thirdReuslt.done).toBe(true);
+    expect(thirdReuslt.value).toBe(undefined);
   });
 
   test('reset iterable when option.reset passed', async () => {
     const initialPath = '/foo';
     const initialParmas = { a: { b: 'c' } };
 
-    // @ts-ignore
     const iterable = gateway.paginate(initialPath, initialParmas);
     const response = {
       headers: {
@@ -394,39 +408,48 @@ describe('Gateway', () => {
     };
 
     ((mockAxios.request as any) as jest.Mock).mockResolvedValue(response);
-    iterable.next();
-    iterable.next({ reset: true });
+    await iterable.next();
+    await iterable.next({ reset: true });
 
+    expect(mockAxios.request).toBeCalledTimes(2);
     expect(mockAxios.request).toHaveBeenLastCalledWith(
       expect.objectContaining({
-        url: gateway.uri + initialPath,
+        url: initialPath,
         params: initialParmas,
       }),
     );
   });
 
-  test('finish pagination with given value', async () => {
-    const iterable = gateway.paginate('/');
-    const value = Symbol('hehehe');
-    const result = await iterable.return!(value);
+  test('set next url and next params by calling next with params', async () => {
+    const initialPath = '/foo';
+    const initialParmas = { page: '1' };
 
-    expect(result.done).toBe(true);
-    expect(result.value).toBe(value);
-  });
+    const iterable = gateway.paginate(initialPath, initialParmas);
+    const response = {
+      headers: {
+        link: '<https://example.com/foo2>; rel="next"',
+      },
+      data: {},
+    };
+    ((mockAxios.request as any) as jest.Mock).mockResolvedValue(response);
 
-  test('throw given error in the pagination', () => {
-    const iterable = gateway.paginate('/');
-    const error = new Error('hehehe');
-    expect(iterable.throw!(error)).rejects.toThrow(error);
-  });
+    await iterable.next();
+    expect(mockAxios.request).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        url: initialPath,
+        params: initialParmas,
+      }),
+    );
+
+    const customNextUrl = '/bar';
+    const customNextParmas = { page: '777' };
 
-  test('Symbol.asyncIterator is defined correctly', () => {
-    const iterable = gateway.paginate('/');
-    expect(iterable[Symbol.asyncIterator]()).toEqual(
+    await iterable.next({ url: customNextUrl, params: customNextParmas });
+    expect(mockAxios.request).toBeCalledTimes(2);
+    expect(mockAxios.request).toHaveBeenLastCalledWith(
       expect.objectContaining({
-        next: expect.any(Function),
-        return: expect.any(Function),
-        throw: expect.any(Function),
+        url: customNextUrl,
+        params: customNextParmas,
       }),
     );
   });