Skip to content

Commit

Permalink
Merge pull request #18532 from davelopez/upgrade_openapi_typescript
Browse files Browse the repository at this point in the history
Upgrade `openapi-typescript` to 7.0.2 + swap to `openapi-fetch`
  • Loading branch information
mvdbeek authored Aug 15, 2024
2 parents 23f946e + 30e39ea commit 2dcd285
Show file tree
Hide file tree
Showing 223 changed files with 14,538 additions and 5,405 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ remove-api-schema:
rm _shed_schema.yaml

update-client-api-schema: client-node-deps build-api-schema
$(IN_VENV) cd client && node openapi_to_schema.mjs ../_schema.yaml > src/api/schema/schema.ts && npx prettier --write src/api/schema/schema.ts
$(IN_VENV) cd client && node openapi_to_schema.mjs ../_shed_schema.yaml > ../lib/tool_shed/webapp/frontend/src/schema/schema.ts && npx prettier --write ../lib/tool_shed/webapp/frontend/src/schema/schema.ts
$(IN_VENV) cd client && npx openapi-typescript ../_schema.yaml > src/api/schema/schema.ts && npx prettier --write src/api/schema/schema.ts
$(IN_VENV) cd client && npx openapi-typescript ../_shed_schema.yaml > ../lib/tool_shed/webapp/frontend/src/schema/schema.ts && npx prettier --write ../lib/tool_shed/webapp/frontend/src/schema/schema.ts
$(MAKE) remove-api-schema

lint-api-schema: build-api-schema
Expand Down
99 changes: 80 additions & 19 deletions client/docs/querying-the-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,44 +24,105 @@ If there is no Composable for the API endpoint you are using, try using a (Pinia

### Direct API Calls

- If the type of data you are querying should not be cached, or you just need to update or create new data, you can use the API directly. Make sure to use the **Fetcher** (see below) instead of Axios, as it provides a type-safe interface to the API along with some extra benefits.
- If the type of data you are querying should not be cached, or you just need to update or create new data, you can use the API directly. Make sure to use the **GalaxyApi client** (see below) instead of Axios, as it provides a type-safe interface to the API along with some extra benefits.

## 2. Prefer Fetcher over Axios (when possible)
## 2. Prefer **GalaxyApi client** over Axios (when possible)

- **Use Fetcher with OpenAPI Specs**: If there is an OpenAPI spec for the API endpoint you are using (in other words, there is a FastAPI route defined in Galaxy), always use the Fetcher. It will provide you with a type-safe interface to the API.
- **Use **GalaxyApi client** with OpenAPI Specs**: If there is an OpenAPI spec for the API endpoint you are using (in other words, there is a FastAPI route defined in Galaxy), always use the GalaxyApi client. It will provide you with a type-safe interface to the API.

**Do**

```typescript
import { fetcher } from "@/api/schema";
const datasetsFetcher = fetcher.path("/api/dataset/{id}").method("get").create();
```ts
import { ref, onMounted } from "vue";

const { data: dataset } = await datasetsFetcher({ id: "testID" });
import { GalaxyApi, type HDADetailed } from "@/api";
import { errorMessageAsString } from "@/utils/simple-error";

interface Props {
datasetId: string;
}

const props = defineProps<Props>();

const datasetDetails = ref<HDADetailed>();
const errorMessage = ref<string>();

async function loadDatasetDetails() {
// Your IDE will provide you with autocompletion for the route and all the parameters
const { data, error } = await GalaxyApi().GET("/api/datasets/{dataset_id}", {
params: {
path: {
dataset_id: props.datasetId,
},
query: { view: "detailed" },
},
});

if (error) {
// Handle error here. For example, you can display a message to the user.
errorMessage.value = errorMessageAsString(error);
// Make sure to return here, otherwise `data` will be undefined
return;
}

// Use `data` here. We are casting it to HDADetailed to help the type inference because
// we requested the "detailed" view and this endpoint returns different types depending
// on the view. In general, the correct type will be inferred automatically using the
// API schema so you don't need to (and shouldn't) cast it.
datasetDetails.value = data as HDADetailed;
}

onMounted(() => {
loadDatasetDetails();
});
```

**Don't**

```js
```ts
import { ref, onMounted } from "vue";

import axios from "axios";
import { getAppRoot } from "onload/loadConfig";
import { rethrowSimple } from "utils/simple-error";
import { errorMessageAsString } from "@/utils/simple-error";

interface Props {
datasetId: string;
}

const props = defineProps<Props>();

const datasetDetails = ref<HDADetailed>();
const errorMessage = ref<string>();

async getDataset(datasetId) {
async function loadDatasetDetails() {
// You need to construct the URL yourself
const url = `${getAppRoot()}api/datasets/${datasetId}`;
// You are forced to use a try-catch block to handle errors
// and you may forget to do so.
try {
const response = await axios.get(url);
return response.data;
// This is not type-safe and cannot detect changes in the API schema.
const response = await axios.get(url, {
// You need to know the API parameters, no type inference here.
params: { view: "detailed" },
});
// In this case, you need to cast the response to the correct type
// (as in the previous example), but you will also have to do it in the general
// case because there is no type inference.
datasetDetails = response.data as HDADetailed;
} catch (e) {
rethrowSimple(e);
errorMessage.value = errorMessageAsString(error);
}
}

const dataset = await getDataset("testID");
onMounted(() => {
loadDatasetDetails();
});
```

> **Reason**
>
> The `fetcher` class provides a type-safe interface to the API, and is already configured to use the correct base URL and error handling.
> The `GalaxyApi client` function provides a type-safe interface to the API, and is already configured to use the correct base URL. In addition, it will force you to handle errors properly, and will provide you with a type-safe response object. It uses `openapi-fetch` and you can find more information about it [here](https://openapi-ts.dev/openapi-fetch/).
## 3. Where to put your API queries?

Expand All @@ -79,8 +140,8 @@ If so, you should consider putting the query in a Store. If not, you should cons

If so, you should consider putting it under src/api/<resource>.ts and exporting it from there. This will allow you to reuse the query in multiple places specially if you need to do some extra processing of the data. Also it will help to keep track of what parts of the API are being used and where.

### Should I use the `fetcher` directly or should I write a wrapper function?
### Should I use the `GalaxyApi client` directly or should I write a wrapper function?

- If you **don't need to do any extra processing** of the data, you can use the `fetcher` directly.
- If you **need to do some extra processing**, you should consider writing a wrapper function. Extra processing can be anything from transforming the data to adding extra parameters to the query or omitting some of them, handling conditional types in response data, etc.
- Using a **wrapper function** will help in case we decide to replace the `fetcher` with something else in the future (as we are doing now with _Axios_).
- If you **don't need to do any additional processing** of the data, you **should** use the `GalaxyApi client` directly.
- If you **need to do some additional processing**, then consider writing a wrapper function. Additional processing can be anything from transforming the data to adding extra parameters to the query, providing useful defaults, handling conditional types in response data, etc.
- In any case, please try to avoid modeling your own types if you can use the ones defined in the OpenAPI spec (client/src/api/schema/schema.ts). This will help to keep the codebase consistent and avoid duplication of types or API specification drifting.
47 changes: 47 additions & 0 deletions client/docs/unit-testing/writing-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,50 @@ describe("some module you wrote", () => {

We have created some [common helpers for common testing
scenarios](https://github.com/galaxyproject/galaxy/blob/dev/client/tests/jest/helpers.js).

### Mocking API calls

When testing components that make API calls, you should use [**Mock Service Worker**](https://mswjs.io/docs/getting-started/) in combination with [**openapi-msw**](https://github.com/christoph-fricke/openapi-msw?tab=readme-ov-file#openapi-msw).

If you want to know more about why MSW is a good choice for mocking API calls, you can read [this article](https://mswjs.io/docs/philosophy).

If your component makes an API call, for example to get a particular history, you can mock the response of the API call using the `useServerMock` composable in your test file.

```ts
import { useServerMock } from "@/api/client/__mocks__";

const { server, http } = useServerMock();

describe("MyComponent", () => {
it("should do something with the history", async () => {
// Mock the response of the API call
server.use(
http.get("/api/histories/{history_id}", ({ params, query, response }) => {
// You can use logic to return different responses based on the request
if (query.get("view") === "detailed") {
return response(200).json(TEST_HISTORY_DETAILED);
}

// Or simulate an error
if (params.history_id === "must-fail") {
return response("5XX").json(EXPECTED_500_ERROR, { status: 500 });
}

return response(200).json(TEST_HISTORY_SUMMARY);
})
);

// Your test code here
});
});
```

Using this approach, it will ensure the type safety of the API calls and the responses. If you need to mock API calls that are not defined in the OpenAPI specs, you can use the `http.untyped` variant to mock any API route. Or define an untyped response for a specific route with `HttpResponse`. See the example below:

```ts
const catchAll = http.untyped.all("/resource/*", ({ params }) => {
return HttpResponse.json(/* ... */);
});
```

For more information on how to use `openapi-msw`, you can check the [official documentation](https://github.com/christoph-fricke/openapi-msw?tab=readme-ov-file#handling-unknown-paths).
21 changes: 0 additions & 21 deletions client/openapi_to_schema.mjs

This file was deleted.

8 changes: 6 additions & 2 deletions client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@
"markdown-it": "^13.0.2",
"markdown-it-regexp": "^0.4.0",
"object-hash": "^3.0.0",
"openapi-typescript": "^6.7.6",
"openapi-typescript-fetch": "^1.1.3",
"openapi-fetch": "^0.10.6",
"pinia": "^2.1.7",
"popper.js": "^1.16.1",
"pretty-bytes": "^6.1.1",
Expand Down Expand Up @@ -172,15 +171,20 @@
"eslint-plugin-vue": "^9.17.0",
"eslint-plugin-vuejs-accessibility": "^2.2.0",
"expose-loader": "^4.1.0",
"fake-indexeddb": "^6.0.0",
"gulp": "^4.0.2",
"ignore-loader": "^0.1.2",
"imports-loader": "^4.0.1",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"jest-fixed-jsdom": "^0.0.2",
"jest-location-mock": "^2.0.0",
"jsdom-worker": "^0.3.0",
"json-loader": "^0.5.7",
"mini-css-extract-plugin": "^2.7.6",
"msw": "^2.3.4",
"openapi-msw": "^0.7.0",
"openapi-typescript": "^7.3.0",
"postcss-loader": "^7.3.3",
"prettier": "^2.8.8",
"process": "^0.11.10",
Expand Down
68 changes: 68 additions & 0 deletions client/src/api/client/__mocks__/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { HttpResponse } from "msw";
import { setupServer } from "msw/node";
import { createOpenApiHttp } from "openapi-msw";

import { type GalaxyApiPaths } from "@/api/schema";

export { HttpResponse };

function createApiClientMock() {
return createOpenApiHttp<GalaxyApiPaths>({ baseUrl: window.location.origin });
}

let http: ReturnType<typeof createApiClientMock>;
let server: ReturnType<typeof setupServer>;

/**
* Returns a `server` instance that can be used to mock the Galaxy API server
* and make requests to the Galaxy API using the OpenAPI schema.
*
* It is an instance of Mock Service Worker (MSW) server (https://github.com/mswjs/msw).
* And the `http` object is an instance of OpenAPI-MSW (https://github.com/christoph-fricke/openapi-msw)
* that add support for full type inference from OpenAPI schema definitions.
*/
export function useServerMock() {
if (!server) {
server = setupServer();
http = createApiClientMock();
}

beforeAll(() => {
// Enable API mocking before all the tests.
server.listen({
onUnhandledRequest: (request) => {
const method = request.method.toLowerCase();
const apiPath = request.url.replace(window.location.origin, "");
const errorMessage = `
No request handler found for ${request.method} ${request.url}.
Make sure you have added a request handler for this request in your tests.
Example:
const { server, http } = useServerMock();
server.use(
http.${method}('${apiPath}', ({ response }) => {
return response(200).json({});
})
);
`;
throw new Error(errorMessage);
},
});
});

afterEach(() => {
// Reset the request handlers between each test.
// This way the handlers we add on a per-test basis
// do not leak to other, irrelevant tests.
server.resetHandlers();
});

afterAll(() => {
// Finally, disable API mocking after the tests are done.
server.close();
});

return { server, http };
}
32 changes: 32 additions & 0 deletions client/src/api/client/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import createClient from "openapi-fetch";

import { type GalaxyApiPaths } from "@/api/schema";
import { getAppRoot } from "@/onload/loadConfig";

function getBaseUrl() {
const isTest = process.env.NODE_ENV === "test";
return isTest ? window.location.origin : getAppRoot(undefined, true);
}

function apiClientFactory() {
return createClient<GalaxyApiPaths>({ baseUrl: getBaseUrl() });
}

export type GalaxyApiClient = ReturnType<typeof apiClientFactory>;

let client: GalaxyApiClient;

/**
* Returns the Galaxy API client.
*
* It can be used to make requests to the Galaxy API using the OpenAPI schema.
*
* See: https://openapi-ts.dev/openapi-fetch/
*/
export function GalaxyApi(): GalaxyApiClient {
if (!client) {
client = apiClientFactory();
}

return client;
}
Loading

0 comments on commit 2dcd285

Please sign in to comment.