From 51c307af25996e3bded9d1b8d4d9b726ce845060 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 29 Apr 2024 14:38:49 +1000 Subject: [PATCH] chore: fix PR reviews --- .../leftpane/LeftPaneSettingSection.tsx | 6 +-- ts/components/settings/SessionSettings.tsx | 2 +- .../settings/SessionSettingsHeader.tsx | 2 +- ts/session/apis/seed_node_api/SeedNodeAPI.ts | 26 +++++------ .../apis/snode_api/SnodeRequestTypes.ts | 44 ++++++++++++++----- .../apis/snode_api/getServiceNodesList.ts | 3 -- ts/session/apis/snode_api/retrieveRequest.ts | 6 +-- ts/state/ducks/settings.tsx | 2 +- ts/types/ReduxTypes.d.ts | 2 +- 9 files changed, 53 insertions(+), 40 deletions(-) diff --git a/ts/components/leftpane/LeftPaneSettingSection.tsx b/ts/components/leftpane/LeftPaneSettingSection.tsx index a05912a78..8f154d157 100644 --- a/ts/components/leftpane/LeftPaneSettingSection.tsx +++ b/ts/components/leftpane/LeftPaneSettingSection.tsx @@ -77,7 +77,7 @@ const getCategories = (): Array<{ id: SessionSettingCategory; title: string }> = title: window.i18n('recoveryPhrase'), }, { - id: 'ClearData' as const, + id: 'clearData' as const, title: window.i18n('clearDataSettingsTitle'), }, ]; @@ -93,7 +93,7 @@ const LeftPaneSettingsCategoryRow = (props: { const dataTestId = `${title.toLowerCase().replace(' ', '-')}-settings-menu-item`; - const isClearData = id === 'ClearData'; + const isClearData = id === 'clearData'; return ( ; // these three down there have no options, they are just a button - case 'ClearData': + case 'clearData': case 'messageRequests': case 'recoveryPhrase': default: diff --git a/ts/components/settings/SessionSettingsHeader.tsx b/ts/components/settings/SessionSettingsHeader.tsx index 91b0cefe5..f2ed4646f 100644 --- a/ts/components/settings/SessionSettingsHeader.tsx +++ b/ts/components/settings/SessionSettingsHeader.tsx @@ -44,7 +44,7 @@ export const SettingsHeader = (props: Props) => { case 'privacy': categoryTitle = window.i18n('privacySettingsTitle'); break; - case 'ClearData': + case 'clearData': case 'messageRequests': case 'recoveryPhrase': throw new Error(`no header for should be tried to be rendered for "${category}"`); diff --git a/ts/session/apis/seed_node_api/SeedNodeAPI.ts b/ts/session/apis/seed_node_api/SeedNodeAPI.ts index a28f03eb2..899b26f4d 100644 --- a/ts/session/apis/seed_node_api/SeedNodeAPI.ts +++ b/ts/session/apis/seed_node_api/SeedNodeAPI.ts @@ -12,6 +12,7 @@ import { allowOnlyOneAtATime } from '../../utils/Promise'; import { APPLICATION_JSON } from '../../../types/MIME'; import { isLinux } from '../../../OS'; import { Snode } from '../../../data/data'; +import { GetServicesNodesFromSeedRequest } from '../snode_api/SnodeRequestTypes'; /** * Fetch all snodes from seed nodes. @@ -228,25 +229,20 @@ async function getSnodesFromSeedUrl(urlObj: URL): Promise> { // we get all active nodes window?.log?.info(`getSnodesFromSeedUrl starting with ${urlObj.href}`); - const params = { - active_only: true, - // If you are thinking of adding the `limit` field here: don't. - // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. - // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. - fields: { - public_ip: true, - storage_port: true, - pubkey_x25519: true, - pubkey_ed25519: true, - }, - }; - const endpoint = 'json_rpc'; const url = `${urlObj.href}${endpoint}`; - const body = { + const body: GetServicesNodesFromSeedRequest = { jsonrpc: '2.0', method: 'get_n_service_nodes', - params, + params: { + active_only: true, + fields: { + public_ip: true, + storage_port: true, + pubkey_x25519: true, + pubkey_ed25519: true, + }, + }, }; const sslAgent = await getSslAgentForSeedNode( diff --git a/ts/session/apis/snode_api/SnodeRequestTypes.ts b/ts/session/apis/snode_api/SnodeRequestTypes.ts index 8b54cfd7f..ee2a7dbb7 100644 --- a/ts/session/apis/snode_api/SnodeRequestTypes.ts +++ b/ts/session/apis/snode_api/SnodeRequestTypes.ts @@ -67,22 +67,42 @@ export type OnsResolveSubRequest = { }; }; +/** + * If you are thinking of adding the `limit` field here: don't. + * We fetch the full list because we will remove from every cached swarms the snodes not found in that fresh list. + * If a `limit` was set, we would remove a lot of valid snodes from those cached swarms. + */ +type FetchSnodeListParams = { + active_only: true; + fields: { + public_ip: true; + storage_port: true; + pubkey_x25519: true; + pubkey_ed25519: true; + }; +}; + +export type GetServicesNodesFromSeedRequest = { + method: 'get_n_service_nodes'; + jsonrpc: '2.0'; + /** + * If you are thinking of adding the `limit` field here: don't. + * We fetch the full list because we will remove from every cached swarms the snodes not found in that fresh list. + * If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. + */ + params: FetchSnodeListParams; +}; + export type GetServiceNodesSubRequest = { method: 'oxend_request'; params: { endpoint: 'get_service_nodes'; - params: { - active_only: true; - // If you are thinking of adding the `limit` field here: don't. - // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. - // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. - fields: { - public_ip: true; - storage_port: true; - pubkey_x25519: true; - pubkey_ed25519: true; - }; - }; + /** + * If you are thinking of adding the `limit` field here: don't. + * We fetch the full list because we will remove from every cached swarms the snodes not found in that fresh list. + * If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. + */ + params: FetchSnodeListParams; }; }; diff --git a/ts/session/apis/snode_api/getServiceNodesList.ts b/ts/session/apis/snode_api/getServiceNodesList.ts index 6c85050a3..8f35d8288 100644 --- a/ts/session/apis/snode_api/getServiceNodesList.ts +++ b/ts/session/apis/snode_api/getServiceNodesList.ts @@ -13,9 +13,6 @@ function buildSnodeListRequests(): Array { endpoint: 'get_service_nodes', params: { active_only: true, - // If you are thinking of adding the `limit` field here: don't. - // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. - // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. fields: { public_ip: true, storage_port: true, diff --git a/ts/session/apis/snode_api/retrieveRequest.ts b/ts/session/apis/snode_api/retrieveRequest.ts index 53f078eb0..50d94eada 100644 --- a/ts/session/apis/snode_api/retrieveRequest.ts +++ b/ts/session/apis/snode_api/retrieveRequest.ts @@ -5,7 +5,7 @@ import { doSnodeBatchRequest } from './batchRequest'; import { GetNetworkTime } from './getNetworkTime'; import { SnodeNamespace, SnodeNamespaces } from './namespaces'; -import { TTL_DEFAULT } from '../../constants'; +import { DURATION, TTL_DEFAULT } from '../../constants'; import { UserUtils } from '../../utils'; import { sleepFor } from '../../utils/Promise'; import { @@ -124,7 +124,7 @@ async function retrieveNextMessages( ); // let exceptions bubble up // no retry for this one as this a call we do every few seconds while polling for messages - const timeOutMs = 10 * 1000; // yes this is a long timeout for just messages, but 4s timeouts way to often... + const timeOutMs = 10 * DURATION.SECONDS; // yes this is a long timeout for just messages, but 4s timeouts way to often... const timeoutPromise = async () => sleepFor(timeOutMs); const fetchPromise = async () => doSnodeBatchRequest(retrieveRequestsParams, targetNode, timeOutMs, associatedWith); @@ -166,7 +166,7 @@ async function retrieveNextMessages( GetNetworkTime.handleTimestampOffsetFromNetwork('retrieve', bodyFirstResult.t); - // NOTE: We don't want to sort messages here because the ordering depends on the snode and when it received each messages. + // NOTE: We don't want to sort messages here because the ordering depends on the snode and when it received each message. // The last_hash for that snode has to be the last one we've received from that same snode, othwerwise we end up fetching the same messages over and over again. return results.map((result, index) => ({ code: result.code, diff --git a/ts/state/ducks/settings.tsx b/ts/state/ducks/settings.tsx index dd89ac1b2..7397b47c3 100644 --- a/ts/state/ducks/settings.tsx +++ b/ts/state/ducks/settings.tsx @@ -1,7 +1,7 @@ import { isBoolean } from 'lodash'; import { PayloadAction, createSlice } from '@reduxjs/toolkit'; -import { SettingsKey } from '../../data/settings-key'; // ok: does not import anything else +import { SettingsKey } from '../../data/settings-key'; const SettingsBoolsKeyTrackedInRedux = [ SettingsKey.someDeviceOutdatedSyncing, diff --git a/ts/types/ReduxTypes.d.ts b/ts/types/ReduxTypes.d.ts index 41195a658..d744b0f14 100644 --- a/ts/types/ReduxTypes.d.ts +++ b/ts/types/ReduxTypes.d.ts @@ -12,7 +12,7 @@ export type SessionSettingCategory = | 'permissions' | 'help' | 'recoveryPhrase' - | 'ClearData'; + | 'clearData'; export type PasswordAction = 'set' | 'change' | 'remove' | 'enter';