This is an automated email from the ASF dual-hosted git repository.
dengliming pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu-dashboard.git
The following commit(s) were added to refs/heads/master by this push:
new 2e355d79 fix: handle config import and export callbacks (#661)
2e355d79 is described below
commit 2e355d79ca205db89f0c7ee6c2aeb2e0a3355da6
Author: joyboy <[email protected]>
AuthorDate: Sun Sep 27 09:57:38 2026 +0530
fix: handle config import and export callbacks (#661)
* fix: handle config import and export callbacks
* addressing review comments, adding tests for download and handling it
better
---------
Co-authored-by: Mukesh Babu <[email protected]>
---
src/components/GlobalHeader/index.js | 9 +-
src/components/GlobalHeader/parseImportResult.js | 20 ++++
.../GlobalHeader/parseImportResult.test.js | 30 +++++
src/models/common.js | 16 ++-
src/models/common.test.js | 126 +++++++++++++++++++++
src/utils/download.js | 31 ++++-
src/utils/download.test.js | 110 ++++++++++++++++++
7 files changed, 336 insertions(+), 6 deletions(-)
diff --git a/src/components/GlobalHeader/index.js
b/src/components/GlobalHeader/index.js
index 5e5824f8..4584de43 100644
--- a/src/components/GlobalHeader/index.js
+++ b/src/components/GlobalHeader/index.js
@@ -22,6 +22,7 @@ import { withRouter } from "dva/router";
import ImportModal from "./ImportModal";
import ExportModal from "./ExportModal";
import ImportResultModal from "./ImportResultModal";
+import parseImportResult from "./parseImportResult";
import styles from "./index.less";
import { getCurrentLocale, getIntlContent } from "../../utils/IntlUtils";
import { checkUserPassword } from "../../services/api";
@@ -188,7 +189,10 @@ class GlobalHeader extends PureComponent {
payload: values,
callback: (res) => {
this.closeModal(true);
- this.showImportRestlt(JSON.parse(res));
+ const importResult = parseImportResult(res);
+ if (importResult) {
+ this.showImportRestlt(importResult);
+ }
},
});
}}
@@ -232,9 +236,8 @@ class GlobalHeader extends PureComponent {
dispatch({
type: "common/exportByNamespace",
payload: values,
- callback: (res) => {
+ callback: () => {
this.closeModal(true);
- this.showImportRestlt(JSON.parse(res));
},
});
}}
diff --git a/src/components/GlobalHeader/parseImportResult.js
b/src/components/GlobalHeader/parseImportResult.js
new file mode 100644
index 00000000..cd94f0d0
--- /dev/null
+++ b/src/components/GlobalHeader/parseImportResult.js
@@ -0,0 +1,20 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export default function parseImportResult(result) {
+ return result ? JSON.parse(result) : null;
+}
diff --git a/src/components/GlobalHeader/parseImportResult.test.js
b/src/components/GlobalHeader/parseImportResult.test.js
new file mode 100644
index 00000000..ce386cea
--- /dev/null
+++ b/src/components/GlobalHeader/parseImportResult.test.js
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import parseImportResult from "./parseImportResult";
+
+describe("parseImportResult", () => {
+ it("returns null when a successful import has no detail", () => {
+ expect(parseImportResult()).toBeNull();
+ });
+
+ it("parses import details when the backend returns them", () => {
+ expect(parseImportResult('{"pluginImportSuccessCount":1}')).toEqual({
+ pluginImportSuccessCount: 1,
+ });
+ });
+});
diff --git a/src/models/common.js b/src/models/common.js
index 7935bbff..fff87f26 100644
--- a/src/models/common.js
+++ b/src/models/common.js
@@ -254,11 +254,23 @@ export default {
},
*exportAll(_, { call }) {
- yield call(asyncConfigExport);
+ try {
+ yield call(asyncConfigExport);
+ } catch (error) {
+ message.error(error.message);
+ }
},
*exportByNamespace(params, { call }) {
- yield call(asyncConfigExportByNamespace, params);
+ const { callback } = params;
+ try {
+ yield call(asyncConfigExportByNamespace, params);
+ if (callback) {
+ callback();
+ }
+ } catch (error) {
+ message.error(error.message);
+ }
},
*import(params, { call }) {
diff --git a/src/models/common.test.js b/src/models/common.test.js
new file mode 100644
index 00000000..ea237559
--- /dev/null
+++ b/src/models/common.test.js
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { message } from "antd";
+import fetch from "dva/fetch";
+
+jest.mock("antd", () => ({
+ message: {
+ error: jest.fn(),
+ success: jest.fn(),
+ warn: jest.fn(),
+ },
+}));
+
+jest.mock("dva/fetch", () => jest.fn());
+jest.mock("../utils/request", () => jest.fn());
+
+document.body.innerHTML = '<div id="httpPath"></div>';
+
+const common = require("./common").default;
+const { asyncConfigExportByNamespace } = require("../services/api");
+
+const createErrorResponse = ({ ok, status, statusText, body }) => ({
+ ok,
+ status,
+ statusText,
+ headers: {
+ get: jest.fn(() => null),
+ },
+ json: jest.fn().mockResolvedValue(body),
+});
+
+describe("common export effects", () => {
+ it("invokes its callback after the export completes", () => {
+ const callback = jest.fn();
+ const params = {
+ payload: { namespace: "default" },
+ callback,
+ };
+ const call = jest.fn((fn, args) => ({ fn, args }));
+ const iterator = common.effects.exportByNamespace(params, { call });
+
+ expect(iterator.next().value).toEqual({
+ fn: asyncConfigExportByNamespace,
+ args: params,
+ });
+ expect(callback).not.toHaveBeenCalled();
+
+ expect(iterator.next().done).toBe(true);
+ expect(callback).toHaveBeenCalledTimes(1);
+ });
+
+ it.each([
+ [
+ "an HTTP error",
+ createErrorResponse({
+ ok: false,
+ status: 401,
+ statusText: "Unauthorized",
+ body: { code: 401, message: "Authentication failed", data: null },
+ }),
+ "Authentication failed",
+ ],
+ [
+ "an HTTP 200 Admin error",
+ createErrorResponse({
+ ok: true,
+ status: 200,
+ statusText: "OK",
+ body: { code: 601, message: "Export failed", data: null },
+ }),
+ "Export failed",
+ ],
+ ])("keeps the modal open for %s", async (_, response, expectedMessage) => {
+ const callback = jest.fn();
+ const params = {
+ payload: { namespace: "default" },
+ callback,
+ };
+ const call = jest.fn((fn, args) => fn(args));
+ const iterator = common.effects.exportByNamespace(params, { call });
+ fetch.mockResolvedValue(response);
+
+ const exportRequest = iterator.next().value;
+ let exportError;
+ try {
+ await exportRequest;
+ } catch (error) {
+ exportError = error;
+ }
+
+ expect(exportError).toBeInstanceOf(Error);
+ const result = iterator.throw(exportError);
+
+ expect(result.done).toBe(true);
+ expect(callback).not.toHaveBeenCalled();
+ expect(message.error).toHaveBeenCalledWith(
+ expect.stringContaining(expectedMessage),
+ );
+ });
+
+ it("surfaces failures when exporting all configuration", () => {
+ const call = jest.fn((fn, args) => ({ fn, args }));
+ const iterator = common.effects.exportAll({}, { call });
+
+ iterator.next();
+ const result = iterator.throw(new Error("Export failed"));
+
+ expect(result.done).toBe(true);
+ expect(message.error).toHaveBeenCalledWith("Export failed");
+ });
+});
diff --git a/src/utils/download.js b/src/utils/download.js
index fbe18f78..9da35238 100644
--- a/src/utils/download.js
+++ b/src/utils/download.js
@@ -17,6 +17,25 @@
import fetch from "dva/fetch";
+async function getDownloadErrorMessage(response) {
+ const fallbackMessage =
+ !response.ok && response.statusText
+ ? response.statusText
+ : `Export failed (${response.status})`;
+
+ try {
+ const result = await response.json();
+ if (result && result.message) {
+ return result.message;
+ }
+ return result && result.code
+ ? `Export failed (${result.code})`
+ : fallbackMessage;
+ } catch (error) {
+ return fallbackMessage;
+ }
+}
+
/**
* Requests a URL, for downloading.
*
@@ -42,6 +61,16 @@ export default async function download(url, options) {
try {
const response = await fetch(url, newOptions);
const disposition = response.headers.get("Content-Disposition");
+ const isAttachment =
+ response.ok &&
+ disposition &&
+ disposition.toLowerCase().includes("attachment");
+
+ if (!isAttachment) {
+ const errorMessage = await getDownloadErrorMessage(response);
+ throw new Error(errorMessage);
+ }
+
const filenameRegex = /filename[^;=\n]*=((['"]).*?\2|[^;\n]*)/;
const matches = filenameRegex.exec(disposition);
let filename = "download";
@@ -58,6 +87,6 @@ export default async function download(url, options) {
a.click();
document.body.removeChild(a);
} catch (error) {
- throw new Error(`下载文件失败：${error}`);
+ throw new Error(`下载文件失败：${error.message || error}`);
}
}
diff --git a/src/utils/download.test.js b/src/utils/download.test.js
new file mode 100644
index 00000000..e72e9e76
--- /dev/null
+++ b/src/utils/download.test.js
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import fetch from "dva/fetch";
+import download from "./download";
+
+jest.mock("dva/fetch", () => jest.fn());
+
+const createHeaders = (headers = {}) => ({
+ get: jest.fn((name) => headers[name.toLowerCase()] || null),
+});
+
+const createResponse = ({
+ ok = true,
+ status = 200,
+ statusText = "OK",
+ headers = {},
+ json,
+ blob,
+}) => ({
+ ok,
+ status,
+ statusText,
+ headers: createHeaders(headers),
+ json: jest.fn().mockImplementation(() => Promise.resolve(json)),
+ blob: jest.fn().mockImplementation(() => Promise.resolve(blob)),
+});
+
+describe("download", () => {
+ let clickSpy;
+
+ beforeEach(() => {
+ window.sessionStorage.clear();
+ Object.defineProperty(window.URL, "createObjectURL", {
+ configurable: true,
+ value: jest.fn(() => "blob:config"),
+ });
+ clickSpy = jest
+ .spyOn(HTMLAnchorElement.prototype, "click")
+ .mockImplementation(() => {});
+ });
+
+ it("downloads a successful attachment", async () => {
+ const blob = new Blob(["config"], { type: "application/zip" });
+ const response = createResponse({
+ headers: {
+ "content-disposition": 'attachment;filename="config.zip"',
+ },
+ blob,
+ });
+ fetch.mockResolvedValue(response);
+
+ await expect(download("/configs/export", { method: "GET" })).resolves.toBe(
+ undefined,
+ );
+
+ expect(response.blob).toHaveBeenCalledTimes(1);
+ expect(response.json).not.toHaveBeenCalled();
+ expect(window.URL.createObjectURL).toHaveBeenCalledWith(blob);
+ expect(clickSpy).toHaveBeenCalledTimes(1);
+ });
+
+ it("rejects an HTTP error without downloading it", async () => {
+ const response = createResponse({
+ ok: false,
+ status: 401,
+ statusText: "Unauthorized",
+ json: { code: 401, message: "Authentication failed", data: null },
+ });
+ fetch.mockResolvedValue(response);
+
+ await expect(
+ download("/configs/export", { method: "GET" }),
+ ).rejects.toThrow("Authentication failed");
+
+ expect(response.blob).not.toHaveBeenCalled();
+ expect(clickSpy).not.toHaveBeenCalled();
+ });
+
+ it.each([601, 500])(
+ "rejects an HTTP 200 Admin error with code %s",
+ async (code) => {
+ const response = createResponse({
+ json: { code, message: "Export failed", data: null },
+ });
+ fetch.mockResolvedValue(response);
+
+ await expect(
+ download("/configs/export", { method: "GET" }),
+ ).rejects.toThrow("Export failed");
+
+ expect(response.blob).not.toHaveBeenCalled();
+ expect(clickSpy).not.toHaveBeenCalled();
+ },
+ );
+});
