Skip to content

Commit

Permalink
unify interface of http helpers (#1139)
Browse files Browse the repository at this point in the history 
## Problem

Http helpers has inconsistent API.


## Solution

Unify it on standard js Response object.
  • Loading branch information
@jreidinger
jreidinger authored Apr 9, 2024
2 parents ada6305 + 91dd51c commit c5e1c5d
Show file tree
Hide file tree
Showing 10 changed files with 215 additions and 87 deletions.
15 changes: 5 additions & 10 deletions web/src/client/http.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,21 @@ class HTTPClient {

/**
* @param {string} url - Endpoint URL (e.g., "/l10n/config").
* @return {Promise<object>} Server response.
* @return {Promise<Response>} Server response.
*/
async get(url) {
const response = await fetch(`${this.baseUrl}${url}`, {
headers: {
"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<object>} Server response.
* @return {Promise<Response>} Server response.
*/
async post(url, data) {
const response = await fetch(`${this.baseUrl}${url}`, {
Expand All @@ -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<object>} Server response.
* @return {Promise<Response>} Server response.
*/
async put(url, data) {
const response = await fetch(`${this.baseUrl}${url}`, {
Expand Down
48 changes: 39 additions & 9 deletions web/src/client/l10n.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,20 @@ class L10nClient {
* @return {Promise<String>} Locale ID.
*/
async getUILocale() {
const config = await this.client.get("/l10n/config");
const response = await this.client.get("/l10n/config");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return "";
}
const config = await response.json();
return config.uiLocale;
}

/**
* Sets the locale to translate the installer UI.
*
* @param {String} id - Locale ID.
* @return {Promise<void>}
* @return {Promise<Response>}
*/
async setUILocale(id) {
return this.client.patch("/l10n/config", { uiLocale: id });
Expand All @@ -82,7 +87,12 @@ class L10nClient {
* @return {Promise<String>} Keymap ID.
*/
async getUIKeymap() {
const config = await this.client.get("/l10n/config");
const response = await this.client.get("/l10n/config");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return "";
}
const config = await response.json();
return config.uiKeymap;
}

Expand All @@ -92,7 +102,7 @@ class L10nClient {
* This setting is only relevant in the local installation.
*
* @param {String} id - Keymap ID.
* @return {Promise<void>}
* @return {Promise<Response>}
*/
async setUIKeymap(id) {
return this.client.patch("/l10n/config", { uiKeymap: id });
Expand All @@ -104,7 +114,12 @@ class L10nClient {
* @return {Promise<Array<Timezone>>}
*/
async timezones() {
const timezones = await this.client.get("/l10n/timezones");
const response = await this.client.get("/l10n/timezones");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return [];
}
const timezones = await response.json();
return timezones.map(this.buildTimezone);
}

Expand Down Expand Up @@ -136,7 +151,12 @@ class L10nClient {
* @return {Promise<Array<Locale>>}
*/
async locales() {
const locales = await this.client.get("/l10n/locales");
const response = await this.client.get("/l10n/locales");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return [];
}
const locales = await response.json();
return locales.map(this.buildLocale);
}

Expand Down Expand Up @@ -169,7 +189,12 @@ class L10nClient {
* @return {Promise<Array<Keymap>>}
*/
async keymaps() {
const keymaps = await this.client.get("/l10n/keymaps");
const response = await this.client.get("/l10n/keymaps");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return [];
}
const keymaps = await response.json();
return keymaps.map(this.buildKeymap);
}

Expand Down Expand Up @@ -242,7 +267,12 @@ class L10nClient {
* @return {Promise<object>} Localization configuration.
*/
async getConfig() {
return await this.client.get("/l10n/config");
const response = await this.client.get("/l10n/config");
if (!response.ok) {
console.error("Failed to get localization config: ", response);
return {};
}
return await response.json();
}

/**
Expand All @@ -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<object>}
* @return {Promise<Response>}
*/
async setConfig(data) {
return this.client.patch("/l10n/config", data);
Expand Down
31 changes: 23 additions & 8 deletions web/src/client/manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ class ManagerBaseClient {
*
* The progress of the probing process can be tracked through installer signals.
*
* @return {Promise<void>}
* @return {Promise<Response>}
*/
startProbing() {
return this.client.post("/manager/probe");
return this.client.post("/manager/probe", {});
}

/**
* Start the installation process
*
* The progress of the installation process can be tracked through installer signals.
*
* @return {Promise}
* @return {Promise<Response>}
*/
startInstallation() {
return this.client.post("/manager/install");
return this.client.post("/manager/install", {});
}

/**
Expand All @@ -70,7 +70,12 @@ class ManagerBaseClient {
* @return {Promise<boolean>}
*/
async canInstall() {
const installer = await this.client.get("/manager/installer");
const response = await this.client.get("/manager/installer");
if (!response.ok) {
console.error("Failed to get installer config: ", response);
return false;
}
const installer = await response.json();
return installer.canInstall;
}

Expand All @@ -90,7 +95,12 @@ class ManagerBaseClient {
* @return {Promise<number>}
*/
async getPhase() {
const installer = await this.client.get("/manager/installer");
const response = await this.client.get("/manager/installer");
if (!response.ok) {
console.error("Failed to get installer config: ", response);
return 0;
}
const installer = await response.json();
return installer.phase;
}

Expand All @@ -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", {});
}

/**
Expand All @@ -121,7 +131,12 @@ class ManagerBaseClient {
* @return {Promise<boolean>}
*/
async useIguana() {
const installer = await this.client.get("/manager/installer");
const response = await this.client.get("/manager/installer");
if (!response.ok) {
console.error("Failed to get installer config: ", response);
return false;
}
const installer = await response.json();
return installer.iguana;
}
}
Expand Down
64 changes: 43 additions & 21 deletions web/src/client/mixins.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,14 @@ const WithIssues = (superclass, issues_path, dbus_path) =>
* @return {Promise<Issue[]>}
*/
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);
}
}

/**
Expand Down Expand Up @@ -137,8 +143,14 @@ const WithStatus = (superclass, status_path, service_name) =>
* @return {Promise<number>} 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;
}
}

/**
Expand Down Expand Up @@ -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,
};
}
}

/**
Expand Down Expand Up @@ -252,15 +273,16 @@ const WithValidation = (superclass, validation_path, service_name) => class exte
* @return {Promise<ValidationError[]>}
*/
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);
}

/**
Expand Down
17 changes: 12 additions & 5 deletions web/src/client/network.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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);
Expand All @@ -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 }]);
});
Expand All @@ -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");
});
Expand Down
Loading

0 comments on commit c5e1c5d

Please sign in to comment.