Skip to content
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

Refactor client API packages #16847

Merged
merged 34 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
32b3a6d
Move client `schema` package into `api/schema`
davelopez Oct 13, 2023
8968828
Add client api/jobs module
davelopez Oct 13, 2023
41473d5
Add client api/users module
davelopez Oct 13, 2023
7cce9e9
Add client api/notifications.broadcast module
davelopez Oct 13, 2023
9c5549c
Add client api/notifications module
davelopez Oct 13, 2023
75bd1d7
Move getUsers to api/users and rename
davelopez Oct 13, 2023
57afdfb
Move getRoles to api/roles and rename
davelopez Oct 13, 2023
60b8c34
Move getGroups to api/groups and rename
davelopez Oct 13, 2023
04b6e77
Move client dataset services to api/datasets
davelopez Oct 13, 2023
b53889b
Move client datatypes services to api/datatypes
davelopez Oct 13, 2023
ce3b2f2
Move client remote files services to api/remoteFIles
davelopez Oct 13, 2023
42df8dd
Move client history export services to src/api
davelopez Oct 13, 2023
baae447
Move client object stores services to src/api
davelopez Oct 13, 2023
0f0b611
Move client tags services to src/api
davelopez Oct 16, 2023
a905bb6
Move client pages services to src/api
davelopez Oct 16, 2023
8ed098a
Fix PageDropdown tests
davelopez Oct 16, 2023
a924588
Move client notification preferences services to src/api
davelopez Oct 16, 2023
bde1a7b
Consolidate datatypes imports
davelopez Oct 17, 2023
71ff69b
Add client api/dbKeys module
davelopez Oct 17, 2023
9743e66
Consolidate remote files fetcher imports
davelopez Oct 17, 2023
4583427
Rename refactoring in Disk Usage Management
davelopez Oct 17, 2023
a5161dc
Move histories and datasets fetchers to their own modules
davelopez Oct 17, 2023
0ef52a2
Remove duplicated fetcher
davelopez Oct 17, 2023
c6f10f3
Consolidate notifications broadcasts client API
davelopez Oct 17, 2023
a9a8fa3
Move common API type alias to api/index
davelopez Oct 19, 2023
93e8332
Move fetchDatasetDetails to api/datasets
davelopez Oct 19, 2023
a9141aa
Move collection services to src/api
davelopez Oct 19, 2023
391738c
Reuse fetchCollectionDetails
davelopez Oct 19, 2023
7d9d67f
Move archived histories services to src/api
davelopez Oct 20, 2023
b679da9
Move History related type alias imports
davelopez Oct 20, 2023
9f16be7
Move more notification queries to api/notifications
davelopez Oct 20, 2023
65cea29
Fix minor typing issue
davelopez Oct 20, 2023
506a3a6
Use `import type` for types
davelopez Oct 20, 2023
9913b71
Add first draft of client API guide
davelopez Oct 23, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ area/toolshed:
- lib/toolshed/**/*
- templates/webapps/tool_shed/**/*
area/UI-UX:
- all: ["client/src/**/*", "!client/src/schema/schema.ts"]
- all: ["client/src/**/*", "!client/src/api/schema/schema.ts"]
any: ["templates/**/*"]
area/util:
- lib/galaxy/util/**/*
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint_openapi_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
- name: Check for changes
run: |
if [[ `git status --porcelain` ]]; then
echo "Rebuilding client/src/schema/schema.ts resulted in changes, run 'make update-client-api-schema' and commit results"
echo "Rebuilding client/src/api/schema/schema.ts resulted in changes, run 'make update-client-api-schema' and commit results"
exit 1
fi
working-directory: 'galaxy root'
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ 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/schema/schema.ts && npx prettier --write src/schema/schema.ts
$(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
$(MAKE) remove-api-schema

Expand Down
86 changes: 86 additions & 0 deletions client/docs/querying-the-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Best practices when querying the API from UI components

If you need to query the API from a component, there are several ways to do so. This document will help you decide which one to use and provide some best practices when doing so to keep the code clean and maintainable.

## Choose the Right Approach

When querying APIs in Vue components, consider the following approaches and their best practices:

## 1. Prefer Composables over Stores over Direct API Calls

### Composables

- **What Are Composables?**: please read the official Vue documentation on composables [here](https://vuejs.org/guide/reusability/composables.html).

If there is already a composable that takes care of the logic you need for accessing a particular resource in the API please use it or consider writing a new one. They provide a type-safe interface and a higher level of abstraction than the related Store or the API itself. They might rely on one or more Stores for caching and reactivity.

### Stores

- **Stores Explained**: Please read the official Vue documentation on State Management [here](https://vuejs.org/guide/scaling-up/state-management.html).

If there is no Composable for the API endpoint you are using, try using a (Pinia) Store instead. Stores are type-safe and provide a reactive interface to the API. They can also be used to cache data, ensuring a single source of truth.

- **Use Pinia Stores**: If you need to create a new Store, make sure to create a Pinia Store, and not a Vuex Store as Pinia will be the replacement for Vuex. Also, use [Composition API syntax over the Options API](https://vuejs.org/guide/extras/composition-api-faq.html) for increased readability, code organization, type inference, etc.

### 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.

## 2. Prefer Fetcher 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.

**Do**

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

const { data: dataset } = await datasetsFetcher({ id: "testID" });
```

**Don't**

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

async getDataset(datasetId) {
const url = `${getAppRoot()}api/datasets/${datasetId}`;
try {
const response = await axios.get(url);
return response.data;
} catch (e) {
rethrowSimple(e);
}
}

const dataset = await getDataset("testID");
```

> **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.

## 3. Where to put your API queries?

The short answer is: **it depends**. There are several factors to consider when deciding where to put your API queries:

### Is the data you are querying related exclusively to a particular component?

If so, you should put the query in the component itself. If not, you should consider putting it in a Composable or a Store.

### Can the data be cached?

If so, you should consider putting the query in a Store. If not, you should consider putting it in a Composable or the component itself.

### Is the query going to be used in more than one place?

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?

- 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_).
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { fetcher } from "@/schema";

import { CollectionEntry, DatasetCollectionAttributes, DCESummary, HDCADetailed, isHDCA } from ".";
import { CollectionEntry, DatasetCollectionAttributes, DCESummary, HDCADetailed, isHDCA } from "@/api";
import { fetcher } from "@/api/schema";

const DEFAULT_LIMIT = 50;

const getCollectionDetails = fetcher.path("/api/dataset_collections/{id}").method("get").create();

export async function fetchCollectionDetails(params: { hdcaId: string }): Promise<HDCADetailed> {
const { data } = await getCollectionDetails({ id: params.hdcaId });
/**
* Fetches the details of a collection.
* @param params.id The ID of the collection (HDCA) to fetch.
*/
export async function fetchCollectionDetails(params: { id: string }): Promise<HDCADetailed> {
const { data } = await getCollectionDetails({ id: params.id });
return data;
}

Expand Down
89 changes: 89 additions & 0 deletions client/src/api/datasets.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import type { FetchArgType } from "openapi-typescript-fetch";

import { DatasetDetails } from "@/api";
import { fetcher } from "@/api/schema";
import { withPrefix } from "@/utils/redirect";

export const datasetsFetcher = fetcher.path("/api/datasets").method("get").create();

type GetDatasetsApiOptions = FetchArgType<typeof datasetsFetcher>;
type GetDatasetsQuery = Pick<GetDatasetsApiOptions, "limit" | "offset">;
// custom interface for how we use getDatasets
interface GetDatasetsOptions extends GetDatasetsQuery {
sortBy?: string;
sortDesc?: string;
query?: string;
}

/** Datasets request helper **/
export async function getDatasets(options: GetDatasetsOptions = {}) {
const params: GetDatasetsApiOptions = {};
if (options.sortBy) {
const sortPrefix = options.sortDesc ? "-dsc" : "-asc";
params.order = `${options.sortBy}${sortPrefix}`;
}
if (options.limit) {
params.limit = options.limit;
}
if (options.offset) {
params.offset = options.offset;
}
if (options.query) {
params.q = ["name-contains"];
params.qv = [options.query];
}
const { data } = await datasetsFetcher(params);
return data;
}

const getDataset = fetcher.path("/api/datasets/{dataset_id}").method("get").create();

export async function fetchDatasetDetails(params: { id: string }): Promise<DatasetDetails> {
const { data } = await getDataset({ dataset_id: params.id, view: "detailed" });
// We know that the server will return a DatasetDetails object because of the view parameter
// but the type system doesn't, so we have to cast it.
return data as unknown as DatasetDetails;
}

const updateHistoryDataset = fetcher.path("/api/histories/{history_id}/contents/{type}s/{id}").method("put").create();

export async function undeleteHistoryDataset(historyId: string, datasetId: string) {
const { data } = await updateHistoryDataset({
history_id: historyId,
id: datasetId,
type: "dataset",
deleted: false,
});
return data;
}

const deleteHistoryDataset = fetcher
.path("/api/histories/{history_id}/contents/{type}s/{id}")
.method("delete")
.create();

export async function purgeHistoryDataset(historyId: string, datasetId: string) {
const { data } = await deleteHistoryDataset({ history_id: historyId, id: datasetId, type: "dataset", purge: true });
return data;
}

const datasetCopy = fetcher.path("/api/histories/{history_id}/contents/{type}s").method("post").create();
type HistoryContentsArgs = FetchArgType<typeof datasetCopy>;
export async function copyDataset(
datasetId: HistoryContentsArgs["content"],
historyId: HistoryContentsArgs["history_id"],
type: HistoryContentsArgs["type"] = "dataset",
source: HistoryContentsArgs["source"] = "hda"
) {
const response = await datasetCopy({
history_id: historyId,
type,
source: source,
content: datasetId,
});
return response.data;
}

export function getCompositeDatasetLink(historyDatasetId: string, path: string) {
return withPrefix(`/api/datasets/${historyDatasetId}/display?filename=${path}`);
}
13 changes: 13 additions & 0 deletions client/src/api/datatypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { fetcher } from "@/api/schema";

export const datatypesFetcher = fetcher.path("/api/datatypes").method("get").create();

export const edamFormatsFetcher = fetcher.path("/api/datatypes/edam_formats/detailed").method("get").create();
export const edamDataFetcher = fetcher.path("/api/datatypes/edam_data/detailed").method("get").create();

const typesAndMappingsFetcher = fetcher.path("/api/datatypes/types_and_mapping").method("get").create();

export async function fetchDatatypesAndMappings(upload_only = true) {
const { data } = await typesAndMappingsFetcher({ upload_only });
return data;
}
8 changes: 8 additions & 0 deletions client/src/api/dbKeys.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Historically, this API was used to get the list of genomes that were available
* but now it is used to get the list of more generic "dbkeys".
*/

import { fetcher } from "@/api/schema";

export const dbKeysFetcher = fetcher.path("/api/genomes").method("get").create();
9 changes: 9 additions & 0 deletions client/src/api/groups.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import axios from "axios";

import { components } from "@/api/schema";

type GroupModel = components["schemas"]["GroupModel"];
export async function getAllGroups(): Promise<GroupModel[]> {
const { data } = await axios.get("/api/groups");
return data;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { FetchArgType } from "openapi-typescript-fetch";

import { type components, fetcher } from "@/schema";
import { type components, fetcher } from "@/api/schema";

export type ArchivedHistorySummary = components["schemas"]["ArchivedHistorySummary"];
export type ArchivedHistoryDetailed = components["schemas"]["ArchivedHistoryDetailed"];
Expand Down Expand Up @@ -55,7 +55,7 @@
};
}

const archiveHistory = fetcher.path("/api/histories/{history_id}/archive").method("post").create();
const postArchiveHistory = fetcher.path("/api/histories/{history_id}/archive").method("post").create();

/**
* Archive a history.
Expand All @@ -64,23 +64,23 @@
* @param purgeHistory Whether to purge the history after archiving. Can only be used in combination with an archive export record.
* @returns The archived history summary.
*/
export async function archiveHistoryById(
export async function archiveHistory(
historyId: string,
archiveExportId?: string,
purgeHistory?: boolean
): Promise<ArchivedHistorySummary> {
const { data } = await archiveHistory({
const { data } = await postArchiveHistory({
history_id: historyId,
archive_export_id: archiveExportId,
purge_history: purgeHistory,
});
return data as ArchivedHistorySummary;
}

const unarchiveHistory = fetcher
const putUnarchiveHistory = fetcher
.path("/api/histories/{history_id}/archive/restore")
.method("put")
// @ts-ignore: workaround for optional query parameters in PUT. More info here https://github.com/ajaishankar/openapi-typescript-fetch/pull/55

Check warning on line 83 in client/src/api/histories.archived.ts

View workflow job for this annotation

GitHub Actions / client-unit-test (18)

Do not use "@ts-ignore" because it alters compilation errors
.create({ force: undefined });

/**
Expand All @@ -89,8 +89,8 @@
* @param force Whether to force un-archiving for purged histories.
* @returns The restored history summary.
*/
export async function unarchiveHistoryById(historyId: string, force?: boolean): Promise<ArchivedHistorySummary> {
const { data } = await unarchiveHistory({ history_id: historyId, force });
export async function unarchiveHistory(historyId: string, force?: boolean): Promise<ArchivedHistorySummary> {
const { data } = await putUnarchiveHistory({ history_id: historyId, force });
return data as ArchivedHistorySummary;
}

Expand All @@ -102,7 +102,7 @@
* @param archivedHistory The archived history to reimport. It must have an associated export record.
* @returns The async task result summary to track the reimport progress.
*/
export async function reimportHistoryFromExportRecordAsync(
export async function reimportArchivedHistoryFromExportRecord(
archivedHistory: ArchivedHistorySummary
): Promise<AsyncTaskResultSummary> {
if (!archivedHistory.export_record_data) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { components } from "@/api/schema";
import { fetcher } from "@/api/schema";
import type { ObjectExportTaskResponse } from "@/components/Common/models/exportRecordModel";
import { ExportRecordModel } from "@/components/Common/models/exportRecordModel";
import { DEFAULT_EXPORT_PARAMS } from "@/composables/shortTermStorage";
import type { components } from "@/schema";
import { fetcher } from "@/schema";

type ModelStoreFormat = components["schemas"]["ModelStoreFormat"];

Expand All @@ -24,7 +24,7 @@ export const AVAILABLE_EXPORT_FORMATS: { id: ModelStoreFormat; name: string }[]
* @param params query and pagination params
* @returns a promise with a list of export records associated with the given history.
*/
export async function getExportRecords(historyId: string) {
export async function fetchHistoryExportRecords(historyId: string) {
const response = await _getExportRecords(
{
history_id: historyId,
Expand All @@ -46,7 +46,7 @@ export async function getExportRecords(historyId: string) {
* @param exportParams additional parameters to configure the export
* @returns A promise with the request response
*/
export async function exportToFileSource(
export async function exportHistoryToFileSource(
historyId: string,
exportDirectory: string,
fileName: string,
Expand Down
6 changes: 6 additions & 0 deletions client/src/api/histories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { fetcher } from "@/api/schema";

export const historiesFetcher = fetcher.path("/api/histories").method("get").create();
export const archivedHistoriesFetcher = fetcher.path("/api/histories/archived").method("get").create();
export const undeleteHistory = fetcher.path("/api/histories/deleted/{history_id}/undelete").method("post").create();
export const purgeHistory = fetcher.path("/api/histories/{history_id}").method("delete").create();
14 changes: 13 additions & 1 deletion client/src/stores/services/index.ts → client/src/api/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
import { components } from "@/schema";
/** Contains type alias and definitions related to Galaxy API models. */

import { components } from "@/api/schema";

/**
* Contains minimal information about a History.
*/
export type HistorySummary = components["schemas"]["HistorySummary"];

/**
* Contains additional details about a History.
*/
export type HistoryDetailed = components["schemas"]["HistoryDetailed"];

/**
* Contains minimal information about a HistoryContentItem.
Expand Down
4 changes: 4 additions & 0 deletions client/src/api/jobs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { fetcher } from "@/api/schema";

export const jobLockStatus = fetcher.path("/api/job_lock").method("get").create();
export const jobLockUpdate = fetcher.path("/api/job_lock").method("put").create();
Loading
Loading