openapi-ts icon indicating copy to clipboard operation
openapi-ts copied to clipboard

Remove custom request file support

Open mrlubos opened this issue 1 year ago • 14 comments

Describe the problem

Maintaining API is hard

Describe the proposed solution

We should support all use cases for custom requests to remove the need for this feature

Alternatives considered

No response

Importance

cannot use project without

Additional information

No response

mrlubos avatar Mar 27 '24 01:03 mrlubos

That's a nice idea but you probably really want to make sure that you cover a lot of ground, from the top of my head I can think of at least three cases that forced me to use a custom request file:

  • track some header values from one request to another: this can now easily be implemented using the interceptors added in 0.30.0, so 👍
  • support binary responses: looking at the code I can see that your fork does already support that as well, so 👍 but there should probably be a way to expand the list of mime types patterns considered as binary content (application/pdf for example).
  • support for catching 409 Conflict errors
  • there's probably more...

lucsky avatar Mar 27 '24 15:03 lucsky

@lucsky are you able to share your custom request?

mrlubos avatar Mar 27 '24 15:03 mrlubos

@mrlubos Ideally I would provide PRs but I need to go through my company's OSS contribution approval process first... 😅

lucsky avatar Mar 28 '24 08:03 lucsky

@lucsky that would be great if possible. Which client do you use (fetch?)? The list of mime types considered binary can be expanded for sure, if you have some recommended ones to add I can do so. Im also curious why we do not already support 409 errors, seems like a possible bug.

jordanshatford avatar Mar 28 '24 09:03 jordanshatford

@jordanshatford Sorry, I forgot to mention that: yes I'm using the fetch client.

lucsky avatar Mar 28 '24 09:03 lucsky

@lucsky are you able to ditch the custom request file with v0.32.0?

mrlubos avatar Mar 30 '24 17:03 mrlubos

@mrlubos I have just put a PR up to better handle content type. After which I would propose we remove custom request file support and see what tickets come from it.

jordanshatford avatar Mar 31 '24 08:03 jordanshatford

@mrlubos I should have time in the next few days to upgrade a couple repos to the latest openapi-ts, the addition of a few more binary types in 0.33.0 should allow me to completely ditch the custom request file. I'll keep you posted.

Thanks for all the work!

lucsky avatar Apr 02 '24 07:04 lucsky

and see what tickets come from it.

Here's just one ahead :-)

First: Thank you for this project, it's good to have a successor for the obviously unmaintained openapi-typescript-codegen. Changing our existing codebase to @hey-api/openapi-ts was a five minute task.

We currently use this in a legacy AngularJS (i.e. 1.x) application. To work with AngularJS' digest and update mechanism and to allow us to continue using AngularJS specific HTTP interceptors, we have a customized request script which essentially delegates all requests to AngularJS' $http service. This is crude for sure, but has worked well enough so that we didn't further bother with it until today.

Without the custom requests we'd probably need to do some fundamental refactor - which I totally do understand from your maintenance perspective. I just wanted to bring some attention that there are likely some quite exotic use cases out there :-)

Again - thank you for your efforts of bringing this back to life!

qqilihq avatar Apr 02 '24 08:04 qqilihq

@qqilihq thanks for the heads up. We will definitely take that into account before removing custom requests. Would it be possible to share your custom request file? If not no worries, but it may be helpful to us.

jordanshatford avatar Apr 02 '24 08:04 jordanshatford

@qqilihq I second that, are you able to share the custom file? We actually discussed this with Jordan before regarding supporting Angular versions, we could offer it as another legacy client option

mrlubos avatar Apr 02 '24 13:04 mrlubos

Hey there,

thanks for the friendly feedback and sure, here’s how our current code looks like.

  1. There’s an ApiService which creates the client and which passes some parameters to the generated requests file:
import * as angular from 'angular';
import { BaseHttpRequest, CancelablePromise, MyApiClient } from '../api';
import { ApiRequestOptions } from '../api/core/ApiRequestOptions';
import { AngularRequestConfig, request } from '../api/core/request';

export type IApiService = IApiServiceFactory & MyApiClient;

interface IApiServiceFactory {
  /** Create a new client instance which is customized
   * with the given request configuration; we use this
   * to pass custom parameters to the interceptors
   * (e.g. `ignoreLoadingBar`), etc. */
  (angularRequestConfig?: AngularRequestConfig): MyApiClient;
}

angular.module('app').factory('ApiService', [
  '$rootScope',
  '$http',
  '$q',
  ($rootScope: angular.IRootScopeService, $http: angular.IHttpService, $q: angular.IQService): IApiService => {
    const factory: IApiServiceFactory = (angularRequestConfig = {}) =>
      new MyApiClient(
        { BASE: '/api' },
        class extends BaseHttpRequest {
          request<T>(options: ApiRequestOptions): CancelablePromise<T> {
            return request<T>(this.config, options, $rootScope, $http, $q, angularRequestConfig);
          }
        }
      );

    const defaultInstance = factory();

    // return a combination of factory and instance
    // with default configuration for convenience
    return Object.assign(factory, defaultInstance);
  },
]);
  1. The customized request code looks like this. Afair this is based on the fetch code:
/* istanbul ignore file */

/* tslint:disable */

/* eslint-disable */
import * as angular from 'angular';
import { ApiError } from './ApiError';
import type { ApiRequestOptions } from './ApiRequestOptions';
import type { ApiResult } from './ApiResult';
import { CancelablePromise } from './CancelablePromise';
import type { OpenAPIConfig } from './OpenAPI';

// ************************************************
// This code is based on the auto-generated `request.ts`,
// and modified to use AngularJS’s $http service, instead
// of a plain XHR - this way, we can continue to use
// AngularJS HTTP interceptors, …
// ************************************************

const isDefined = <T>(value: T | null | undefined): value is Exclude<T, null | undefined> => {
  return value !== undefined && value !== null;
};

const isString = (value: any): value is string => {
  return typeof value === 'string';
};

const isStringWithValue = (value: any): value is string => {
  return isString(value) && value !== '';
};

const isBlob = (value: any): value is Blob => {
  return (
    typeof value === 'object' &&
    typeof value.type === 'string' &&
    typeof value.stream === 'function' &&
    typeof value.arrayBuffer === 'function' &&
    typeof value.constructor === 'function' &&
    typeof value.constructor.name === 'string' &&
    /^(Blob|File)$/.test(value.constructor.name) &&
    /^(Blob|File)$/.test(value[Symbol.toStringTag])
  );
};

const isFormData = (value: any): value is FormData => {
  return value instanceof FormData;
};

const isSuccess = (status: number): boolean => {
  return status >= 200 && status < 300;
};

const base64 = (str: string): string => {
  try {
    return btoa(str);
  } catch (err) {
    // @ts-ignore
    return Buffer.from(str).toString('base64');
  }
};

const getQueryString = (params: Record<string, any>): string => {
  const qs: string[] = [];

  const append = (key: string, value: any) => {
    qs.push(`${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`);
  };

  const process = (key: string, value: any) => {
    if (isDefined(value)) {
      if (Array.isArray(value)) {
        value.forEach((v) => {
          process(key, v);
        });
      } else if (typeof value === 'object') {
        Object.entries(value).forEach(([k, v]) => {
          process(`${key}[${k}]`, v);
        });
      } else {
        append(key, value);
      }
    }
  };

  Object.entries(params).forEach(([key, value]) => {
    process(key, value);
  });

  if (qs.length > 0) {
    return `?${qs.join('&')}`;
  }

  return '';
};

const getUrl = (config: OpenAPIConfig, options: ApiRequestOptions): string => {
  const encoder = config.ENCODE_PATH || encodeURI;

  const path = options.url
    .replace('{api-version}', config.VERSION)
    .replace(/{(.*?)}/g, (substring: string, group: string) => {
      if (options.path?.hasOwnProperty(group)) {
        return encoder(String(options.path[group]));
      }
      return substring;
    });

  const url = `${config.BASE}${path}`;
  if (options.query) {
    return `${url}${getQueryString(options.query)}`;
  }
  return url;
};

const getFormData = (options: ApiRequestOptions): FormData | undefined => {
  if (options.formData) {
    const formData = new FormData();

    const process = (key: string, value: any) => {
      if (isString(value) || isBlob(value)) {
        formData.append(key, value);
      } else {
        formData.append(key, JSON.stringify(value));
      }
    };

    Object.entries(options.formData)
      .filter(([_, value]) => isDefined(value))
      .forEach(([key, value]) => {
        if (Array.isArray(value)) {
          value.forEach((v) => process(key, v));
        } else {
          process(key, value);
        }
      });

    return formData;
  }
  return undefined;
};

type Resolver<T> = (options: ApiRequestOptions) => Promise<T>;

const resolve = async <T>(options: ApiRequestOptions, resolver?: T | Resolver<T>): Promise<T | undefined> => {
  if (typeof resolver === 'function') {
    return (resolver as Resolver<T>)(options);
  }
  return resolver;
};

const getHeaders = async (config: OpenAPIConfig, options: ApiRequestOptions): Promise<Record<string, string>> => {
  const token = await resolve(options, config.TOKEN);
  const username = await resolve(options, config.USERNAME);
  const password = await resolve(options, config.PASSWORD);
  const additionalHeaders = await resolve(options, config.HEADERS);

  const headers = Object.entries({
    Accept: 'application/json',
    ...additionalHeaders,
    ...options.headers,
  })
    .filter(([_, value]) => isDefined(value))
    .reduce(
      (headers, [key, value]) => ({
        ...headers,
        [key]: String(value),
      }),
      {} as Record<string, string>
    );

  if (isStringWithValue(token)) {
    headers['Authorization'] = `Bearer ${token}`;
  }

  if (isStringWithValue(username) && isStringWithValue(password)) {
    const credentials = base64(`${username}:${password}`);
    headers['Authorization'] = `Basic ${credentials}`;
  }

  if (options.body) {
    if (options.mediaType) {
      headers['Content-Type'] = options.mediaType;
    } else if (isBlob(options.body)) {
      headers['Content-Type'] = options.body.type || 'application/octet-stream';
    } else if (isString(options.body)) {
      headers['Content-Type'] = 'text/plain';
    } else if (!isFormData(options.body)) {
      headers['Content-Type'] = 'application/json';
    }
  } else if (options.formData) {
    // workaround for `FST_ERR_CTP_INVALID_CONTENT_LENGTH` -
    // this prevents setting a `Content-Type: application/json;charset=utf-8`
    // for multipart form data requests
    headers['Content-Type'] = undefined;
  }

  return headers;
};

const getRequestBody = (options: ApiRequestOptions): any => {
  if (options.body) {
    if (options.mediaType?.includes('/json')) {
      return JSON.stringify(options.body);
    } else if (isString(options.body) || isBlob(options.body) || isFormData(options.body)) {
      return options.body;
    } else {
      return JSON.stringify(options.body);
    }
  }

  return undefined;
};

const getResponseHeader = (
  httpResponse: angular.IHttpResponse<unknown>,
  responseHeader?: string
): string | undefined => {
  if (responseHeader) {
    const content = httpResponse.headers(responseHeader);
    if (isString(content)) {
      return content;
    }
  }
  return undefined;
};

const getResponseBody = (httpResponse: angular.IHttpResponse<unknown>): any => {
  return httpResponse.status !== 204 ? httpResponse.data : undefined;
};

const catchErrorCodes = (options: ApiRequestOptions, result: ApiResult): void => {
  const errors: Record<number, string> = {
    400: 'Bad Request',
    401: 'Unauthorized',
    403: 'Forbidden',
    404: 'Not Found',
    500: 'Internal Server Error',
    502: 'Bad Gateway',
    503: 'Service Unavailable',
    ...options.errors,
  };

  const error = errors[result.status];
  if (error) {
    throw new ApiError(options, result, error);
  }

  if (!result.ok) {
    throw new ApiError(options, result, 'Generic Error');
  }
};

/** AngularJS HTTP request config (except `method` and `url` which are given). */
export type AngularRequestConfig = Omit<angular.IRequestConfig, 'method' | 'url'>;

/**
 * Request method
 * @param config The OpenAPI configuration object
 * @param options The request options from the service
 * @returns CancelablePromise<T>
 * @throws ApiError
 */
export const request = <T>(
  config: OpenAPIConfig,
  options: ApiRequestOptions,
  // these are optional to not break `XHRHttpRequest.ts`
  // (which in fact is not needed)
  $rootScope?: angular.IRootScopeService,
  $http?: angular.IHttpService,
  $q?: angular.IQService,
  requestConfig?: AngularRequestConfig
): CancelablePromise<T> => {
  if (!$http || !requestConfig || !$rootScope || !$q) {
    throw new Error();
  }
  return new CancelablePromise(async (resolve, reject, onCancel) => {
    try {
      const url = getUrl(config, options);
      const formData = getFormData(options);
      const body = getRequestBody(options);
      const headers = await getHeaders(config, options);

      if (!onCancel.isCancelled) {
        const cancel = $q.defer();
        onCancel(() => cancel.resolve());
        const response = await $http({
          url,
          method: options.method,
          withCredentials: config.WITH_CREDENTIALS,
          headers,
          data: body ?? formData,
          timeout: cancel.promise,
          ...requestConfig,
        });
        const responseBody = getResponseBody(response);
        const responseHeader = getResponseHeader(response, options.responseHeader);

        const result: ApiResult = {
          url,
          ok: isSuccess(response.status),
          status: response.status,
          statusText: response.statusText,
          body: responseHeader ?? responseBody,
        };

        catchErrorCodes(options, result);

        resolve(result.body);
      }
    } catch (error) {
      reject(error);
    } finally {
      // this is the crucial part
      // $timeout(() => $rootScope.$apply());
      // https://stackoverflow.com/questions/17301572/whats-the-difference-between-evalasync-and-timeout-in-angularjs
      $rootScope.$evalAsync();
    }
  });
};
  1. We use it like this:
class MyController {
  static $inject = ['ApiService'];

  constructor(private readonly apiService: IApiService) {}

  suggestItems(value: unknown) {
    if (typeof value !== 'string') return;
    if (!value.length) return;
    return this.apiService.items
      .listItems({ q: value, limit: 10, sort: 'slug' })
      .then((result) => result.items);
  }
}

Not sure if supporting such old Angular versions out of the box makes sense, but I'm generally happy to provide any kind of support or contributions if it helps!

qqilihq avatar Apr 02 '24 14:04 qqilihq

Hey all, I just published the new Fetch API client (demo), would love to hear your thoughts! In a similar fashion, a separate client will be published for Axios, etc

mrlubos avatar May 21 '24 12:05 mrlubos

Thank you for the update @mrlubos - if I understand correctly, this would allow us to move the AngularJS code which I posted above into its own client module when updating to the latest openapi-ts version?

qqilihq avatar Jun 05 '24 10:06 qqilihq