From f5532043f01806128be0eb202deee12528f53e84 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 9 Apr 2024 10:11:26 +0200 Subject: [PATCH 1/3] unify interface of http helpers --- web/src/client/http.js | 15 +++------ web/src/client/l10n.js | 48 ++++++++++++++++++++++------ web/src/client/manager.js | 31 +++++++++++++----- web/src/client/mixins.js | 64 +++++++++++++++++++++++++------------ web/src/client/questions.js | 7 +++- web/src/client/software.js | 37 +++++++++++++++------ web/src/client/users.js | 27 ++++++++++------ 7 files changed, 161 insertions(+), 68 deletions(-) diff --git a/web/src/client/http.js b/web/src/client/http.js index 844c6545a1..96e1045637 100644 --- a/web/src/client/http.js +++ b/web/src/client/http.js @@ -95,7 +95,7 @@ class HTTPClient { /** * @param {string} url - Endpoint URL (e.g., "/l10n/config"). - * @return {Promise} Server response. + * @return {Promise} Server response. */ async get(url) { const response = await fetch(`${this.baseUrl}${url}`, { @@ -103,13 +103,13 @@ class HTTPClient { "Content-Type": "application/json", }, }); - return await response.json(); + return response; } /** * @param {string} url - Endpoint URL (e.g., "/l10n/config"). * @param {object} data - Data to submit - * @return {Promise} Server response. + * @return {Promise} Server response. */ async post(url, data) { const response = await fetch(`${this.baseUrl}${url}`, { @@ -120,18 +120,13 @@ class HTTPClient { }, }); - try { - return await response.json(); - } catch (e) { - console.warn("Expecting a JSON response", e); - return response.status === 200; - } + return response; } /** * @param {string} url - Endpoint URL (e.g., "/l10n/config"). * @param {object} data - Data to submit - * @return {Promise} Server response. + * @return {Promise} Server response. */ async put(url, data) { const response = await fetch(`${this.baseUrl}${url}`, { diff --git a/web/src/client/l10n.js b/web/src/client/l10n.js index c816c7d2cb..e6b61f4c39 100644 --- a/web/src/client/l10n.js +++ b/web/src/client/l10n.js @@ -60,7 +60,12 @@ class L10nClient { * @return {Promise} Locale ID. */ async getUILocale() { - const config = await this.client.get("/l10n/config"); + const response = await this.client.get("/l10n/config"); + if (!response.ok) { + console.log("Failed to get localizaation config: ", response); + return ""; + } + const config = await response.json(); return config.uiLocale; } @@ -68,7 +73,7 @@ class L10nClient { * Sets the locale to translate the installer UI. * * @param {String} id - Locale ID. - * @return {Promise} + * @return {Promise} */ async setUILocale(id) { return this.client.patch("/l10n/config", { uiLocale: id }); @@ -82,7 +87,12 @@ class L10nClient { * @return {Promise} Keymap ID. */ async getUIKeymap() { - const config = await this.client.get("/l10n/config"); + const response = await this.client.get("/l10n/config"); + if (!response.ok) { + console.log("Failed to get localizaation config: ", response); + return ""; + } + const config = await response.json(); return config.uiKeymap; } @@ -92,7 +102,7 @@ class L10nClient { * This setting is only relevant in the local installation. * * @param {String} id - Keymap ID. - * @return {Promise} + * @return {Promise} */ async setUIKeymap(id) { return this.client.patch("/l10n/config", { uiKeymap: id }); @@ -104,7 +114,12 @@ class L10nClient { * @return {Promise>} */ async timezones() { - const timezones = await this.client.get("/l10n/timezones"); + const response = await this.client.get("/l10n/timezones"); + if (!response.ok) { + console.log("Failed to get localizaation config: ", response); + return []; + } + const timezones = await response.json(); return timezones.map(this.buildTimezone); } @@ -136,7 +151,12 @@ class L10nClient { * @return {Promise>} */ async locales() { - const locales = await this.client.get("/l10n/locales"); + const response = await this.client.get("/l10n/locales"); + if (!response.ok) { + console.log("Failed to get localizaation config: ", response); + return []; + } + const locales = await response.json(); return locales.map(this.buildLocale); } @@ -169,7 +189,12 @@ class L10nClient { * @return {Promise>} */ async keymaps() { - const keymaps = await this.client.get("/l10n/keymaps"); + const response = await this.client.get("/l10n/keymaps"); + if (!response.ok) { + console.log("Failed to get localizaation config: ", response); + return []; + } + const keymaps = await response.json(); return keymaps.map(this.buildKeymap); } @@ -242,7 +267,12 @@ class L10nClient { * @return {Promise} Localization configuration. */ async getConfig() { - return await this.client.get("/l10n/config"); + const response = await this.client.get("/l10n/config"); + if (!response.ok) { + console.log("Failed to get localization config: ", response); + return {}; + } + return await response.json(); } /** @@ -251,7 +281,7 @@ class L10nClient { * Convenience method to set l10n the configuration. * * @param {object} data - Configuration to update. It can just part of the configuration. - * @return {Promise} + * @return {Promise} */ async setConfig(data) { return this.client.patch("/l10n/config", data); diff --git a/web/src/client/manager.js b/web/src/client/manager.js index 02990abb9b..ffe8eb51be 100644 --- a/web/src/client/manager.js +++ b/web/src/client/manager.js @@ -44,10 +44,10 @@ class ManagerBaseClient { * * The progress of the probing process can be tracked through installer signals. * - * @return {Promise} + * @return {Promise} */ startProbing() { - return this.client.post("/manager/probe"); + return this.client.post("/manager/probe", {}); } /** @@ -55,10 +55,10 @@ class ManagerBaseClient { * * The progress of the installation process can be tracked through installer signals. * - * @return {Promise} + * @return {Promise} */ startInstallation() { - return this.client.post("/manager/install"); + return this.client.post("/manager/install", {}); } /** @@ -70,7 +70,12 @@ class ManagerBaseClient { * @return {Promise} */ async canInstall() { - const installer = await this.client.get("/manager/installer"); + const response = await this.client.get("/manager/installer"); + if (!response.ok) { + console.log("Failed to get installer config: ", response); + return false; + } + const installer = await response.json(); return installer.canInstall; } @@ -90,7 +95,12 @@ class ManagerBaseClient { * @return {Promise} */ async getPhase() { - const installer = await this.client.get("/manager/installer"); + const response = await this.client.get("/manager/installer"); + if (!response.ok) { + console.log("Failed to get installer config: ", response); + return 0; + } + const installer = await response.json(); return installer.phase; } @@ -112,7 +122,7 @@ class ManagerBaseClient { * Runs cleanup when installation is done */ finishInstallation() { - return this.client.post("/manager/finish"); + return this.client.post("/manager/finish", {}); } /** @@ -121,7 +131,12 @@ class ManagerBaseClient { * @return {Promise} */ async useIguana() { - const installer = await this.client.get("/manager/installer"); + const response = await this.client.get("/manager/installer"); + if (!response.ok) { + console.log("Failed to get installer config: ", response); + return false; + } + const installer = await response.json(); return installer.iguana; } } diff --git a/web/src/client/mixins.js b/web/src/client/mixins.js index 6e4fb3d851..d00e1382f0 100644 --- a/web/src/client/mixins.js +++ b/web/src/client/mixins.js @@ -92,8 +92,14 @@ const WithIssues = (superclass, issues_path, dbus_path) => * @return {Promise} */ async getIssues() { - const issues = await this.client.get(issues_path); - return issues.map(buildIssue); + const response = await this.client.get(issues_path); + if (!response.ok) { + console.log("get issues failed with:", response); + return []; + } else { + const issues = await response.json(); + return issues.map(buildIssue); + } } /** @@ -137,8 +143,14 @@ const WithStatus = (superclass, status_path, service_name) => * @return {Promise} 0 for idle, 1 for busy */ async getStatus() { - const status = await this.client.get(status_path); - return status.current; + const response = await this.client.get(status_path); + if (!response.ok) { + console.log("get status failed with:", response); + return 1; // lets use busy to be on safe side + } else { + const status = await response.json(); + return status.current; + } } /** @@ -187,15 +199,24 @@ const WithProgress = (superclass, progress_path, service_name) => * the current step and whether the service finished or not. */ async getProgress() { - const { current_step, max_steps, current_title, finished } = await this.client.get( - progress_path, - ); - return { - total: max_steps, - current: current_step, - message: current_title, - finished, - }; + const response = await this.client.get(progress_path); + if (!response.ok) { + console.log("get progress failed with:", response); + return { + total: 0, + current: 0, + message: "Failed to get progress", + finished: false + }; + } else { + const { current_step, max_steps, current_title, finished } = await response.json(); + return { + total: max_steps, + current: current_step, + message: current_title, + finished, + }; + } } /** @@ -252,15 +273,16 @@ const WithValidation = (superclass, validation_path, service_name) => class exte * @return {Promise} */ async getValidationErrors() { - let response; - - try { - response = await this.client.get(validation_path); - } catch (error) { - console.error(`Could not get validation errors for ${validation_path}`, error); + const response = await this.client.get(validation_path); + if (!response.ok) { + console.log("get validation failed with:", response); + return [{ + message: "Failed to validate", + }]; + } else { + const data = await response.json(); + return data.errors.map(createError); } - - return response.errors.map(createError); } /** diff --git a/web/src/client/questions.js b/web/src/client/questions.js index a8eae7ee98..e21d52855b 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -72,7 +72,12 @@ class QuestionsClient { * @return {Promise>} */ async getQuestions() { - const questions = await this.client.get("/questions"); + const response = await this.client.get("/questions"); + if (!response.ok) { + console.log("Failed to get questions: ", response); + return []; + } + const questions = await response.json(); return questions.map(buildQuestion); } diff --git a/web/src/client/software.js b/web/src/client/software.js index e4f6989837..0375ba2710 100644 --- a/web/src/client/software.js +++ b/web/src/client/software.js @@ -169,7 +169,7 @@ class BaseProductManager { * @returns {string} */ registrationRequirement(value) { - let requirement; + let requirement = "unknown"; switch (value) { case 0: @@ -206,10 +206,10 @@ class SoftwareBaseClient { /** * Asks the service to reload the repositories metadata * - * @return {Promise} + * @return {Promise} */ probe() { - return this.client.post("/software/probe"); + return this.client.post("/software/probe", {}); } /** @@ -217,8 +217,13 @@ class SoftwareBaseClient { * * @return {Promise} */ - getProposal() { - return this.client.get("/software/proposal"); + async getProposal() { + const response = await this.client.get("/software/proposal"); + if (!response.ok) { + console.log("Failed to get software proposal: ", response); + } + + return response.json(); } /** @@ -227,8 +232,13 @@ class SoftwareBaseClient { * @return {Promise} */ async getPatterns() { + const response = await this.client.get("/software/patterns"); + if (!response.ok) { + console.log("Failed to get software patterns: ", response); + return []; + } /** @type Array<{ name: string, category: string, summary: string, description: string, order: string, icon: string }> */ - const patterns = await this.client.get("/software/patterns"); + const patterns = await response.json(); return patterns.map((pattern) => ({ name: pattern.name, category: pattern.category, @@ -249,7 +259,7 @@ class SoftwareBaseClient { /** * @param {Object.} patterns - An object where the keys are the pattern names * and the values whether to install them or not. - * @return {Promise} + * @return {Promise} */ selectPatterns(patterns) { return this.client.put("/software/config", { patterns }); @@ -295,8 +305,11 @@ class ProductBaseClient { * @return {Promise>} */ async getAll() { - const products = await this.client.get("/software/products"); - return products; + const response = await this.client.get("/software/products"); + if (!response.ok) { + console.log("Failed to get software products: ", response); + } + return response.json(); } /** @@ -305,7 +318,11 @@ class ProductBaseClient { * @return {Promise} Selected identifier. */ async getSelected() { - const config = await this.client.get("/software/config"); + const response = await this.client.get("/software/config"); + if (!response.ok) { + console.log("Failed to get software config: ", response); + } + const config = await response.json(); return config.product; } diff --git a/web/src/client/users.js b/web/src/client/users.js index ac570aad53..a62663ba7f 100644 --- a/web/src/client/users.js +++ b/web/src/client/users.js @@ -64,13 +64,12 @@ class UsersBaseClient { * @return {Promise} */ async getUser() { - const user = await this.client.get("/users/first"); - - if (user === null) { + const response = await this.client.get("/users/first"); + if (!response.ok) { + console.log("Failed to get first user config: ", response); return { fullName: "", userName: "", password: "", autologin: false, data: {} }; } - - return user; + return response.json(); } /** @@ -79,8 +78,13 @@ class UsersBaseClient { * @return {Promise} */ async isRootPasswordSet() { - const proxy = await this.client.get("/users/root"); - return proxy.password; + const response = await this.client.get("/users/root"); + if (!response.ok) { + console.log("Failed to get root config: ", response); + return false; + } + const config = await response.json(); + return config.password; } /** @@ -130,8 +134,13 @@ class UsersBaseClient { * @return {Promise} SSH public key or an empty string if it is not set */ async getRootSSHKey() { - const proxy = await this.client.get("/users/root"); - return proxy.sshkey; + const response = await this.client.get("/users/root"); + if (!response.ok) { + console.log("Failed to get root config: ", response); + return ""; + } + const config = await response.json(); + return config.sshkey; } /** From 45ebc485ff4382f6ec73c0ee199d5e57be56f0cc Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 9 Apr 2024 16:25:01 +0200 Subject: [PATCH 2/3] adapt also network code and adapt working tests --- web/src/client/l10n.js | 12 +++++----- web/src/client/manager.js | 6 ++--- web/src/client/network.test.js | 17 ++++++++++---- web/src/client/network/index.js | 41 +++++++++++++++++++++++++-------- web/src/client/software.test.js | 15 ++++++++---- 5 files changed, 63 insertions(+), 28 deletions(-) diff --git a/web/src/client/l10n.js b/web/src/client/l10n.js index e6b61f4c39..140842ee94 100644 --- a/web/src/client/l10n.js +++ b/web/src/client/l10n.js @@ -62,7 +62,7 @@ class L10nClient { async getUILocale() { const response = await this.client.get("/l10n/config"); if (!response.ok) { - console.log("Failed to get localizaation config: ", response); + console.error("Failed to get localization config: ", response); return ""; } const config = await response.json(); @@ -89,7 +89,7 @@ class L10nClient { async getUIKeymap() { const response = await this.client.get("/l10n/config"); if (!response.ok) { - console.log("Failed to get localizaation config: ", response); + console.error("Failed to get localizaation config: ", response); return ""; } const config = await response.json(); @@ -116,7 +116,7 @@ class L10nClient { async timezones() { const response = await this.client.get("/l10n/timezones"); if (!response.ok) { - console.log("Failed to get localizaation config: ", response); + console.error("Failed to get localizaation config: ", response); return []; } const timezones = await response.json(); @@ -153,7 +153,7 @@ class L10nClient { async locales() { const response = await this.client.get("/l10n/locales"); if (!response.ok) { - console.log("Failed to get localizaation config: ", response); + console.error("Failed to get localizaation config: ", response); return []; } const locales = await response.json(); @@ -191,7 +191,7 @@ class L10nClient { async keymaps() { const response = await this.client.get("/l10n/keymaps"); if (!response.ok) { - console.log("Failed to get localizaation config: ", response); + console.error("Failed to get localizaation config: ", response); return []; } const keymaps = await response.json(); @@ -269,7 +269,7 @@ class L10nClient { async getConfig() { const response = await this.client.get("/l10n/config"); if (!response.ok) { - console.log("Failed to get localization config: ", response); + console.error("Failed to get localization config: ", response); return {}; } return await response.json(); diff --git a/web/src/client/manager.js b/web/src/client/manager.js index ffe8eb51be..03c691d6cb 100644 --- a/web/src/client/manager.js +++ b/web/src/client/manager.js @@ -72,7 +72,7 @@ class ManagerBaseClient { async canInstall() { const response = await this.client.get("/manager/installer"); if (!response.ok) { - console.log("Failed to get installer config: ", response); + console.error("Failed to get installer config: ", response); return false; } const installer = await response.json(); @@ -97,7 +97,7 @@ class ManagerBaseClient { async getPhase() { const response = await this.client.get("/manager/installer"); if (!response.ok) { - console.log("Failed to get installer config: ", response); + console.error("Failed to get installer config: ", response); return 0; } const installer = await response.json(); @@ -133,7 +133,7 @@ class ManagerBaseClient { async useIguana() { const response = await this.client.get("/manager/installer"); if (!response.ok) { - console.log("Failed to get installer config: ", response); + console.error("Failed to get installer config: ", response); return false; } const installer = await response.json(); diff --git a/web/src/client/network.test.js b/web/src/client/network.test.js index 5834e08dde..aadbbcb345 100644 --- a/web/src/client/network.test.js +++ b/web/src/client/network.test.js @@ -68,6 +68,15 @@ const mockSettings = { networking_enabled: true } +const mockJsonFn = jest.fn(); + +const mockGetFn = jest.fn().mockImplementation(() => { + return { + ok: true, + json: mockJsonFn, + }; +}); + jest.mock("./http", () => { return { HTTPClient: jest.fn().mockImplementation(() => { @@ -78,14 +87,12 @@ jest.mock("./http", () => { }; }); -const mockGetFn = jest.fn(); - describe("NetworkClient", () => { describe("#connections", () => { it("returns the list of active connections from the adapter", async () => { const http = new HTTPClient(new URL(ADDRESS)); const client = new NetworkClient(http); - mockGetFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]); + mockJsonFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]); const connections = await client.connections(); const eth0 = connections.find(c => c.id === "eth0"); expect(eth0).toEqual(mockConnection); @@ -96,7 +103,7 @@ describe("NetworkClient", () => { it("returns the list of addresses", async () => { const http = new HTTPClient(new URL(ADDRESS)); const client = new NetworkClient(http); - mockGetFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]); + mockJsonFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]); const addresses = await client.addresses(); expect(addresses).toEqual([{ address: "192.168.122.100", prefix: 24 }]); }); @@ -107,7 +114,7 @@ describe("NetworkClient", () => { it("returns network general settings", async () => { const http = new HTTPClient(new URL(ADDRESS)); const client = new NetworkClient(http); - mockGetFn.mockResolvedValue(mockSettings); + mockJsonFn.mockResolvedValue(mockSettings); const settings = await client.settings(); expect(settings.hostname).toEqual("localhost.localdomain"); }); diff --git a/web/src/client/network/index.js b/web/src/client/network/index.js index 7cdc257aeb..3eb5e52ffe 100644 --- a/web/src/client/network/index.js +++ b/web/src/client/network/index.js @@ -106,8 +106,13 @@ class NetworkClient { * @return {Promise} */ async connections() { - const connections = await this.client.get("/network/connections"); + const response = await this.client.get("/network/connections"); + if (!response.ok) { + console.error("Failed to get list of connections", response); + return []; + } + const connections = await response.json(); return connections.map(this.fromApiConnection); } @@ -141,9 +146,14 @@ class NetworkClient { * @return {Promise} */ async accessPoints() { - const access_points = await this.client.get("/network/wifi"); - - return access_points.map(ap => { + const response = await this.client.get("/network/wifi"); + if (!response.ok) { + console.error("Failed to get list of APs", response); + return []; + } + const access_points = await response.json(); + + return access_points.map((ap) => { return createAccessPoint({ ssid: ap.ssid, hwAddress: ap.hw_address, @@ -169,7 +179,7 @@ class NetworkClient { * Apply network changes */ async apply() { - return this.client.put("/network/system/apply"); + return this.client.put("/network/system/apply", {}); } /** @@ -205,7 +215,13 @@ class NetworkClient { * @return {Promise} the added connection */ async addConnection(connection) { - return this.client.post("/network/connections", this.toApiConnection(connection)); + const response = await this.client.post("/network/connections", this.toApiConnection(connection)); + if (!response.ok) { + console.error("Failed to post list of connections", response); + return null; + } + + return response.json(); } /** @@ -232,7 +248,7 @@ class NetworkClient { async updateConnection(connection) { const conn = this.toApiConnection(connection); await this.client.put(`/network/connections/${conn.id}`, conn); - return this.apply(); + return (await this.apply()).ok; } /** @@ -246,7 +262,7 @@ class NetworkClient { */ async deleteConnection(id) { await this.client.delete(`/network/connections/${id}`); - return this.apply(); + return (await this.apply()).ok; } /* @@ -266,8 +282,13 @@ class NetworkClient { * * @return {Promise} */ - settings() { - return this.client.get("/network/state"); + async settings() { + const response = await this.client.get("/network/settings"); + if (!response.ok) { + console.error("Failed to get settings", response); + return {}; + } + return response.json(); } } diff --git a/web/src/client/software.test.js b/web/src/client/software.test.js index 5aeeaf6339..c714ef4590 100644 --- a/web/src/client/software.test.js +++ b/web/src/client/software.test.js @@ -25,6 +25,15 @@ import DBusClient from "./dbus"; import { HTTPClient } from "./http"; import { ProductClient, SoftwareClient } from "./software"; +const mockJsonFn = jest.fn(); + +const mockGetFn = jest.fn().mockImplementation(() => { + return { + ok: true, + json: mockJsonFn, + }; +}); + jest.mock("./http", () => { return { HTTPClient: jest.fn().mockImplementation(() => { @@ -35,8 +44,6 @@ jest.mock("./http", () => { }; }); -const mockGetFn = jest.fn(); - jest.mock("./dbus"); const PRODUCT_IFACE = "org.opensuse.Agama.Software1.Product"; @@ -82,7 +89,7 @@ describe("ProductClient", () => { it.only("returns the list of available products", async () => { const http = new HTTPClient(new URL("http://localhost")); const client = new ProductClient(http); - mockGetFn.mockResolvedValue([tumbleweed, microos]); + mockJsonFn.mockResolvedValue([tumbleweed, microos]); const products = await client.getAll(); expect(products).toEqual([ { id: "Tumbleweed", name: "openSUSE Tumbleweed", description: "Tumbleweed is..." }, @@ -95,7 +102,7 @@ describe("ProductClient", () => { it.only("returns the selected product", async () => { const http = new HTTPClient(new URL("http://localhost")); const client = new ProductClient(http); - mockGetFn.mockResolvedValue({ product: "microos" }); + mockJsonFn.mockResolvedValue({ product: "microos" }); const selected = await client.getSelected(); expect(selected).toEqual("microos"); }); From 91dd51c790699a67c769d35de306aa8f3855dc14 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 9 Apr 2024 16:36:30 +0200 Subject: [PATCH 3/3] fix typo --- web/src/client/l10n.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/client/l10n.js b/web/src/client/l10n.js index 140842ee94..2cdb210969 100644 --- a/web/src/client/l10n.js +++ b/web/src/client/l10n.js @@ -89,7 +89,7 @@ class L10nClient { async getUIKeymap() { const response = await this.client.get("/l10n/config"); if (!response.ok) { - console.error("Failed to get localizaation config: ", response); + console.error("Failed to get localization config: ", response); return ""; } const config = await response.json(); @@ -116,7 +116,7 @@ class L10nClient { async timezones() { const response = await this.client.get("/l10n/timezones"); if (!response.ok) { - console.error("Failed to get localizaation config: ", response); + console.error("Failed to get localization config: ", response); return []; } const timezones = await response.json(); @@ -153,7 +153,7 @@ class L10nClient { async locales() { const response = await this.client.get("/l10n/locales"); if (!response.ok) { - console.error("Failed to get localizaation config: ", response); + console.error("Failed to get localization config: ", response); return []; } const locales = await response.json(); @@ -191,7 +191,7 @@ class L10nClient { async keymaps() { const response = await this.client.get("/l10n/keymaps"); if (!response.ok) { - console.error("Failed to get localizaation config: ", response); + console.error("Failed to get localization config: ", response); return []; } const keymaps = await response.json();