-
Notifications
You must be signed in to change notification settings - Fork 351
refactor(base): migrate Base from Flow to TypeScript #4803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bonchevskyi
wants to merge
1
commit into
box:master
Choose a base branch
from
bonchevskyi:refactor/flow-to-ts-base
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,349 @@ | ||
| /* eslint-disable @typescript-eslint/no-explicit-any -- Preserve explicit Flow `any` contracts. */ | ||
| import type { AxiosError } from 'axios'; | ||
| import noop from 'lodash/noop'; | ||
| import Xhr from '../utils/Xhr'; | ||
| import Cache from '../utils/Cache'; | ||
| import UploadsReachability from './uploads/UploadsReachability'; | ||
| import { getTypedFileId } from '../utils/file'; | ||
| import { getBadItemError, getBadPermissionsError } from '../utils/error'; | ||
| import { | ||
| DEFAULT_HOSTNAME_API, | ||
| DEFAULT_HOSTNAME_UPLOAD, | ||
| HTTP_GET, | ||
| HTTP_POST, | ||
| HTTP_PUT, | ||
| HTTP_DELETE, | ||
| } from '../constants'; | ||
| import type { ElementsErrorCallback, APIOptions } from '../common/types/api'; | ||
| import type APICache from '../utils/Cache'; | ||
|
|
||
| type SuccessCallback = (data?: object) => void; | ||
|
|
||
| interface DeleteRequest { | ||
| data?: object; | ||
| errorCallback: ElementsErrorCallback; | ||
| id: string; | ||
| successCallback: Function; | ||
| url: string; | ||
| } | ||
|
|
||
| interface GetRequest { | ||
| errorCallback: ElementsErrorCallback; | ||
| id: string; | ||
| requestData?: object; | ||
| successCallback: Function; | ||
| url?: string; | ||
| } | ||
|
|
||
| interface WriteRequest { | ||
| data: object; | ||
| errorCallback: ElementsErrorCallback; | ||
| id: string; | ||
| successCallback: Function; | ||
| url: string; | ||
| } | ||
|
|
||
| class Base { | ||
| cache: APICache; | ||
|
|
||
| destroyed: boolean; | ||
|
|
||
| xhr: Xhr; | ||
|
|
||
| apiHost: string; | ||
|
|
||
| /** | ||
| * Optional regional metadata host. | ||
| * | ||
| * When set and distinct from `apiHost`, subclasses (currently `Metadata`) | ||
| * route metadata *instance* endpoints to this host while keeping | ||
| * templates, taxonomies, suggestions, options, and queries on `apiHost`. | ||
| * Empty values, or values equal to `apiHost`, are treated as "not set" | ||
| * and resolve to the same URLs as `apiHost` alone. | ||
| * | ||
| * Transitional: this field exists to support the `metadataApiHost` | ||
| * option while the global `apiHost` is not yet regionalized end-to-end. | ||
| * It is expected to be retired in a future major version. | ||
| * | ||
| * @property {?string} | ||
| */ | ||
| metadataApiHost: string | null | undefined; | ||
|
|
||
| uploadHost: string; | ||
|
|
||
| options: APIOptions; | ||
|
|
||
| consoleLog: Function; | ||
|
|
||
| consoleError: Function; | ||
|
|
||
| errorCode: string; | ||
|
|
||
| successCallback: SuccessCallback; | ||
|
|
||
| errorCallback: ElementsErrorCallback; | ||
|
|
||
| uploadsReachability: UploadsReachability; | ||
|
|
||
| /** | ||
| * [constructor] | ||
| * | ||
| * @param {Object} options | ||
| * @param {string} [options.token] - Auth token | ||
| * @param {string} [options.sharedLink] - Shared link | ||
| * @param {string} [options.sharedLinkPassword] - Shared link password | ||
| * @param {string} [options.apiHost] - Api host | ||
| * @param {string} [options.metadataApiHost] - Regional metadata API host | ||
| * used for metadata *instance* endpoints. Templates, taxonomies, | ||
| * suggestions, options, and queries continue to use `apiHost`. Falls | ||
| * back to `apiHost` when undefined, empty, or equal to `apiHost`. | ||
| * @param {string} [options.uploadHost] - Upload host name | ||
| * @return {Base} Base instance | ||
| */ | ||
| constructor(options: APIOptions) { | ||
| this.cache = options.cache || new Cache(); | ||
| this.apiHost = options.apiHost || DEFAULT_HOSTNAME_API; | ||
| this.metadataApiHost = options.metadataApiHost; | ||
| this.uploadHost = options.uploadHost || DEFAULT_HOSTNAME_UPLOAD; | ||
| // @TODO: avoid keeping another copy of data in this.options | ||
| this.options = { | ||
| ...options, | ||
| apiHost: this.apiHost, | ||
| metadataApiHost: this.metadataApiHost, | ||
| uploadHost: this.uploadHost, | ||
| cache: this.cache, | ||
| }; | ||
| this.xhr = new Xhr(this.options); | ||
| this.destroyed = false; | ||
| this.consoleLog = !!options.consoleLog && !!window.console ? window.console.log || noop : noop; | ||
| this.consoleError = !!options.consoleError && !!window.console ? window.console.error || noop : noop; | ||
| this.uploadsReachability = new UploadsReachability(); | ||
| } | ||
|
|
||
| destroy(): void { | ||
| this.xhr.abort(); | ||
| this.destroyed = true; | ||
| } | ||
|
|
||
| /** | ||
| * Asks the API if its destructor has been called | ||
| * | ||
| * @return {boolean} Whether the API has been destroyed | ||
| */ | ||
| isDestroyed(): boolean { | ||
| return this.destroyed; | ||
| } | ||
|
|
||
| /** | ||
| * Checks that our desired API call has sufficient permissions and an item ID | ||
| * | ||
| * @param {string} permissionToCheck - Permission to check | ||
| * @param {Object} permissions - Permissions object | ||
| * @param {string} id - Item id | ||
| * @return {void} | ||
| */ | ||
| checkApiCallValidity(permissionToCheck: string, permissions?: Record<string, unknown>, id?: string): void { | ||
| if (!id || !permissions) { | ||
| throw getBadItemError(); | ||
| } | ||
|
|
||
| const permission = permissions[permissionToCheck]; | ||
| if (!permission) { | ||
| throw getBadPermissionsError(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Builds an API base URL for an arbitrary host, appending the API | ||
| * version suffix (`/2.0`) and tolerating a trailing slash on the host. | ||
| * | ||
| * Shared helper used by `getBaseApiUrl()` and by subclasses that need | ||
| * to derive a `/2.0` URL from a host other than `this.apiHost` (e.g. | ||
| * `Metadata` when `metadataApiHost` is configured). | ||
| * | ||
| * @param {string} host - api host (e.g. "https://api.box.com") | ||
| * @return {string} base api url with `/2.0` suffix | ||
| */ | ||
| buildApiUrl(host: string): string { | ||
| const suffix: string = host.endsWith('/') ? '2.0' : '/2.0'; | ||
| return `${host}${suffix}`; | ||
| } | ||
|
|
||
| /** | ||
| * Base URL for api | ||
| * | ||
| * @return {string} base url | ||
| */ | ||
| getBaseApiUrl(): string { | ||
| return this.buildApiUrl(this.apiHost); | ||
| } | ||
|
|
||
| /** | ||
| * Base URL for api uploads | ||
| * | ||
| * @return {string} base url | ||
| */ | ||
| getBaseUploadUrl(): string { | ||
| const suffix: string = this.uploadHost.endsWith('/') ? 'api/2.0' : '/api/2.0'; | ||
| return `${this.uploadHost}${suffix}`; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the cache instance | ||
| * | ||
| * @return {Cache} cache instance | ||
| */ | ||
| getCache(): APICache { | ||
| return this.cache; | ||
| } | ||
|
|
||
| /** | ||
| * Generic success handler | ||
| * | ||
| * @param {Object} data - The response data | ||
| */ | ||
| successHandler = (data: any): void => { | ||
| if (!this.isDestroyed() && typeof this.successCallback === 'function') { | ||
| this.successCallback(data); | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Generic error handler | ||
| * | ||
| * @param {AxiosError} error - The request error | ||
| */ | ||
| errorHandler = (error: AxiosError<any>): void => { | ||
| if (!this.isDestroyed() && typeof this.errorCallback === 'function') { | ||
| const { response } = error; | ||
|
|
||
| if (response?.data) { | ||
| this.errorCallback(response.data, this.errorCode); | ||
| } else { | ||
| this.errorCallback(error, this.errorCode); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Gets the URL for the API, meant to be overridden | ||
| * @param {string} id - The item id | ||
| * @return {string} The API URL | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars -- Subclasses implement this method using the item ID. | ||
| getUrl(id: string): string { | ||
| // TODO: Implement this method | ||
| throw new Error('Implement me!'); | ||
| } | ||
|
|
||
| /** | ||
| * Formats an API entry for use in components | ||
| * @param {Object} entry - An API response entry | ||
| * @return {*} The formatted entry | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars -- Subclasses implement this method using the API entry. | ||
| format(entry: object): any { | ||
| // TODO: Implement this method | ||
| throw new Error('Implement me!'); | ||
| } | ||
|
|
||
| /** | ||
| * Generic API GET | ||
| * | ||
| * @param {string} id - The file id | ||
| * @param {Function} successCallback - The success callback | ||
| * @param {Function} errorCallback - The error callback | ||
| * @param {Object} requestData - additional request data | ||
| * @param {string} url - API url | ||
| * @returns {Promise} | ||
| */ | ||
| get({ | ||
| id, | ||
| successCallback, | ||
| errorCallback, | ||
| requestData, // Note: this is inconsistent, other methods use `data` | ||
| url, | ||
| }: GetRequest): Promise<any> { | ||
| const apiUrl = url || this.getUrl(id); | ||
| return this.makeRequest(HTTP_GET, id, apiUrl, successCallback, errorCallback, requestData); | ||
| } | ||
|
|
||
| /** | ||
| * Generic API POST | ||
| * | ||
| * @param {string} id - The file id | ||
| * @param {string} url - The url to post to | ||
| * @param {Object} data - The data to post | ||
| * @param {Function} successCallback - The success callback | ||
| * @param {Function} errorCallback - The error callback | ||
| */ | ||
| post({ id, url, data, successCallback, errorCallback }: WriteRequest): Promise<any> { | ||
| return this.makeRequest(HTTP_POST, id, url, successCallback, errorCallback, data); | ||
| } | ||
|
|
||
| /** | ||
| * Generic API PUT | ||
| * | ||
| * @param {string} id - The file id | ||
| * @param {string} url - The url to put to | ||
| * @param {Object} data - The data to put | ||
| * @param {Function} successCallback - The success callback | ||
| * @param {Function} errorCallback - The error callback | ||
| */ | ||
| put({ id, url, data, successCallback, errorCallback }: WriteRequest): Promise<any> { | ||
| return this.makeRequest(HTTP_PUT, id, url, successCallback, errorCallback, data); | ||
| } | ||
|
|
||
| /** | ||
| * Generic API DELETE | ||
| * | ||
| * @param {string} id - The file id | ||
| * @param {string} url - The url of the item to delete | ||
| * @param {Function} successCallback - The success callback | ||
| * @param {Function} errorCallback - The error callback | ||
| * @param {Object} data optional data to delete | ||
| */ | ||
| delete({ id, url, data, successCallback, errorCallback }: DeleteRequest): Promise<any> { | ||
| return this.makeRequest(HTTP_DELETE, id, url, successCallback, errorCallback, data); | ||
| } | ||
|
|
||
| /** | ||
| * Generic API CRUD operations | ||
| * | ||
| * @param {string} method - which REST method to execute (GET, POST, PUT, DELETE) | ||
| * @param {string} id - The file id | ||
| * @param {string} url - The url of the item to operate on | ||
| * @param {Function} successCallback - The success callback | ||
| * @param {Function} errorCallback - The error callback | ||
| * @param {Object} requestData - Optional info to be added to the API call such as params or request body data | ||
| */ | ||
| async makeRequest( | ||
| method: string, | ||
| id: string, | ||
| url: string, | ||
| successCallback: Function, | ||
| errorCallback: ElementsErrorCallback, | ||
| requestData: object = {}, | ||
| ): Promise<void> { | ||
| if (this.isDestroyed()) { | ||
| return; | ||
| } | ||
|
|
||
| this.successCallback = successCallback as SuccessCallback; | ||
| this.errorCallback = errorCallback; | ||
|
|
||
| const xhrMethod: (request: object) => Promise<{ data: any }> = this.xhr[method.toLowerCase()].bind(this.xhr); | ||
| try { | ||
| const { data } = await xhrMethod({ | ||
| id: getTypedFileId(id), | ||
| url, | ||
| ...requestData, | ||
| }); | ||
| this.successHandler(data); | ||
| } catch (error) { | ||
| this.errorHandler(error); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export default Base; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the
metadataApiHostrouting documentation.src/api/Metadata.jsroutes metadata template endpoints throughmetadataApiHost, andsrc/api/__tests__/Metadata.test.jsverifies that behavior. These descriptions state that templates remain onapiHost. A caller can therefore configure the regional host with an incorrect endpoint-scope assumption.src/api/Base.ts#L58-L60: state that metadata template endpoints usemetadataApiHostwhen configured.src/api/Base.ts#L96-L99: apply the same endpoint-scope correction to the constructor parameter documentation.src/api/Base.js.flow#L48-L50: state that metadata template endpoints usemetadataApiHostwhen configured.src/api/Base.js.flow#L110-L113: apply the same endpoint-scope correction to the constructor parameter documentation.Also synchronize the
metadataApiHostdescription insrc/common/types/api.js.📍 Affects 2 files
src/api/Base.ts#L58-L60(this comment)src/api/Base.ts#L96-L99src/api/Base.js.flow#L48-L50src/api/Base.js.flow#L110-L113🤖 Prompt for AI Agents