Merge pull request #1361 from acelaya-forks/feature/predictable-serve…

…r-ids

Feature/predictable server ids

CHANGELOG.md

@@ -4,6 +4,27 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [Unreleased]
+### Added
+* [#1360](https://github.com/shlinkio/shlink-web-client/issues/1360) Added ability for server IDs to be generated based on the server name and URL, instead of generating a random UUID.
+
+    This can improve sharing a predefined set of servers cia servers.json, env vars, or simply export and import your servers in some other device, and then be able to share server URLs which continue working.
+
+    All existing servers will keep their generated IDs in existing devices for backwards compatibility, but newly created servers will use the new approach.
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* *Nothing*
+
+
 ## [4.2.2] - 2024-10-19
 ### Added
 * *Nothing*

README.md

@@ -7,7 +7,7 @@
 [![GitHub license](https://img.shields.io/github/license/shlinkio/shlink-web-client.svg?style=flat-square)](https://github.com/shlinkio/shlink-web-client/blob/main/LICENSE)
 
 [![Mastodon](https://img.shields.io/mastodon/follow/109329425426175098?color=%236364ff&domain=https%3A%2F%2Ffosstodon.org&label=follow&logo=mastodon&logoColor=white&style=flat-square)](https://fosstodon.org/@shlinkio)
-[![Bluesky](https://img.shields.io/badge/follow-shlinkio-0285FF.svg?style=flat-square&logo=bluesky&logoColor=white)](https://bsky.app/profile/shlinkio.bsky.social)
+[![Bluesky](https://img.shields.io/badge/follow-shlinkio-0285FF.svg?style=flat-square&logo=bluesky&logoColor=white)](https://bsky.app/profile/shlink.io)
 [![Paypal Donate](https://img.shields.io/badge/Donate-paypal-blue.svg?style=flat-square&logo=paypal&colorA=cccccc)](https://slnk.to/donate)
 
 A ReactJS-based progressive web application for [Shlink](https://shlink.io).

package.json

@@ -49,7 +49,6 @@
     "react-router-dom": "^6.27.0",
     "reactstrap": "^9.2.3",
     "redux-localstorage-simple": "^2.5.1",
-    "uuid": "^10.0.0",
     "workbox-core": "^7.1.0",
     "workbox-expiration": "^7.1.0",
     "workbox-precaching": "^7.1.0",

scripts/docker/servers_from_env.sh

@@ -4,6 +4,8 @@ set -e
 
 ME=$(basename $0)
 
+# In order to allow people to pre-configure a server in their shlink-web-client instance via env vars, this function
+# dumps a servers.json file based on the values provided via env vars
 setup_single_shlink_server() {
   [ -n "$SHLINK_SERVER_URL" ] || return 0
   [ -n "$SHLINK_SERVER_API_KEY" ] || return 0

src/app/App.tsx

@@ -50,8 +50,8 @@ const App: FCWithDeps<AppProps, AppDeps> = (
   const isHome = location.pathname === '/';
 
   useEffect(() => {
-    // Try to fetch the remote servers if the list is empty at first
-    // We use a ref because we don't care if the servers list becomes empty later
+    // Try to fetch the remote servers if the list is empty during first render.
+    // We use a ref because we don't care if the servers list becomes empty later.
     if (Object.keys(initialServers.current).length === 0) {
       fetchServers();
     }

src/servers/CreateServer.tsx

@@ -8,8 +8,8 @@ import { NoMenuLayout } from '../common/NoMenuLayout';
 import type { FCWithDeps } from '../container/utils';
 import { componentFactory, useDependencies } from '../container/utils';
 import { useGoBack } from '../utils/helpers/hooks';
-import { randomUUID } from '../utils/utils';
 import type { ServerData, ServersMap, ServerWithId } from './data';
+import { ensureUniqueIds } from './helpers';
 import { DuplicatedServersModal } from './helpers/DuplicatedServersModal';
 import type { ImportServersBtnProps } from './helpers/ImportServersBtn';
 import { ServerForm } from './helpers/ServerForm';
@@ -44,12 +44,12 @@ const CreateServer: FCWithDeps<CreateServerProps, CreateServerDeps> = ({ servers
   const [errorImporting, setErrorImporting] = useTimeoutToggle(false, SHOW_IMPORT_MSG_TIME);
   const [isConfirmModalOpen, toggleConfirmModal] = useToggle();
   const [serverData, setServerData] = useState<ServerData>();
-  const saveNewServer = useCallback((theServerData: ServerData) => {
-    const id = randomUUID();
+  const saveNewServer = useCallback((newServerData: ServerData) => {
+    const [newServerWithUniqueId] = ensureUniqueIds(servers, [newServerData]);
 
-    createServers([{ ...theServerData, id }]);
-    navigate(`/server/${id}`);
-  }, [createServers, navigate]);
+    createServers([newServerWithUniqueId]);
+    navigate(`/server/${newServerWithUniqueId.id}`);
+  }, [createServers, navigate, servers]);
   const onSubmit = useCallback((newServerData: ServerData) => {
     setServerData(newServerData);
 

src/servers/helpers/ImportServersBtn.tsx

@@ -6,9 +6,10 @@ import { useCallback, useRef, useState } from 'react';
 import { Button, UncontrolledTooltip } from 'reactstrap';
 import type { FCWithDeps } from '../../container/utils';
 import { componentFactory, useDependencies } from '../../container/utils';
-import type { ServerData, ServersMap } from '../data';
+import type { ServerData, ServersMap, ServerWithId } from '../data';
 import type { ServersImporter } from '../services/ServersImporter';
 import { DuplicatedServersModal } from './DuplicatedServersModal';
+import { dedupServers, ensureUniqueIds } from './index';
 
 export type ImportServersBtnProps = PropsWithChildren<{
   onImport?: () => void;
@@ -18,17 +19,14 @@ export type ImportServersBtnProps = PropsWithChildren<{
 }>;
 
 type ImportServersBtnConnectProps = ImportServersBtnProps & {
-  createServers: (servers: ServerData[]) => void;
+  createServers: (servers: ServerWithId[]) => void;
   servers: ServersMap;
 };
 
 type ImportServersBtnDeps = {
   ServersImporter: ServersImporter
 };
 
-const serversInclude = (servers: ServerData[], { url, apiKey }: ServerData) =>
-  servers.some((server) => server.url === url && server.apiKey === apiKey);
-
 const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBtnDeps> = ({
   createServers,
   servers,
@@ -43,44 +41,45 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
   const [duplicatedServers, setDuplicatedServers] = useState<ServerData[]>([]);
   const [isModalOpen,, showModal, hideModal] = useToggle();
 
-  const serversToCreate = useRef<ServerData[]>([]);
-  const create = useCallback((serversData: ServerData[]) => {
+  const importedServersRef = useRef<ServerWithId[]>([]);
+  const newServersRef = useRef<ServerWithId[]>([]);
+
+  const create = useCallback((serversData: ServerWithId[]) => {
     createServers(serversData);
     onImport();
   }, [createServers, onImport]);
   const onFile = useCallback(
     async ({ target }: ChangeEvent<HTMLInputElement>) =>
       serversImporter.importServersFromFile(target.files?.[0])
-        .then((newServers) => {
-          serversToCreate.current = newServers;
+        .then((importedServers) => {
+          const { duplicatedServers, newServers } = dedupServers(servers, importedServers);
 
-          const existingServers = Object.values(servers);
-          const dupServers = newServers.filter((server) => serversInclude(existingServers, server));
-          const hasDuplicatedServers = !!dupServers.length;
+          importedServersRef.current = ensureUniqueIds(servers, importedServers);
+          newServersRef.current = ensureUniqueIds(servers, newServers);
 
-          if (!hasDuplicatedServers) {
-            create(newServers);
+          if (duplicatedServers.length === 0) {
+            create(importedServersRef.current);
           } else {
-            setDuplicatedServers(dupServers);
+            setDuplicatedServers(duplicatedServers);
             showModal();
           }
         })
         .then(() => {
-          // Reset input after processing file
+          // Reset file input after processing file
           (target as { value: string | null }).value = null;
         })
         .catch(onImportError),
     [create, onImportError, servers, serversImporter, showModal],
   );
 
   const createAllServers = useCallback(() => {
-    create(serversToCreate.current);
+    create(importedServersRef.current);
     hideModal();
-  }, [create, hideModal, serversToCreate]);
+  }, [create, hideModal]);
   const createNonDuplicatedServers = useCallback(() => {
-    create(serversToCreate.current.filter((server) => !serversInclude(duplicatedServers, server)));
+    create(newServersRef.current);
     hideModal();
-  }, [create, duplicatedServers, hideModal]);
+  }, [create, hideModal]);
 
   return (
     <>
@@ -91,7 +90,15 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
         You can create servers by importing a CSV file with <b>name</b>, <b>apiKey</b> and <b>url</b> columns.
       </UncontrolledTooltip>
 
-      <input type="file" accept=".csv" className="d-none" ref={ref} onChange={onFile} aria-hidden />
+      <input
+        type="file"
+        accept=".csv"
+        className="d-none"
+        aria-hidden
+        ref={ref}
+        onChange={onFile}
+        data-testid="csv-file-input"
+      />
 
       <DuplicatedServersModal
         isOpen={isModalOpen}

src/servers/helpers/index.ts

@@ -0,0 +1,85 @@
+import { groupBy } from '@shlinkio/data-manipulation';
+import type { ServerData, ServersMap, ServerWithId } from '../data';
+
+/**
+ * Builds a potentially unique ID for a server, based on concatenating their name and the hostname of their domain, all
+ * in lowercase and replacing invalid URL characters with hyphens.
+ */
+function idForServer(server: ServerData): string {
+  let urlSegment = server.url;
+  try {
+    const { host, pathname } = new URL(urlSegment);
+    urlSegment = host;
+
+    // Remove leading slash from pathname
+    const normalizedPathname = pathname.substring(1);
+
+    // Include pathname in the ID, if not empty
+    if (normalizedPathname.length > 0) {
+      urlSegment = `${urlSegment} ${normalizedPathname}`;
+    }
+  } catch {
+    // If the server URL is not valid, use the value as is
+  }
+
+  return `${server.name} ${urlSegment}`.toLowerCase().replace(/[^a-zA-Z0-9-_.~]/g, '-');
+}
+
+export function serversListToMap(servers: ServerWithId[]): ServersMap {
+  const serversMap: ServersMap = {};
+  servers.forEach((server) => {
+    serversMap[server.id] = server;
+  });
+
+  return serversMap;
+}
+
+const serversInclude = (serversList: ServerData[], { url, apiKey }: ServerData) =>
+  serversList.some((server) => server.url === url && server.apiKey === apiKey);
+
+export type DedupServersResult = {
+  /** Servers which already exist in the reference list */
+  duplicatedServers: ServerData[];
+  /** Servers which are new based on a reference list */
+  newServers: ServerData[];
+};
+
+/**
+ * Given a list of new servers, checks which of them already exist in a servers map, and which don't
+ */
+export function dedupServers(servers: ServersMap, serversToAdd: ServerData[]): DedupServersResult {
+  const serversList = Object.values(servers);
+  const { duplicatedServers = [], newServers = [] } = groupBy(
+    serversToAdd,
+    (server) => serversInclude(serversList, server) ? 'duplicatedServers' : 'newServers',
+  );
+
+  return { duplicatedServers, newServers };
+}
+
+/**
+ * Given a servers map and a list of servers, return the same list of servers but all with an ID, ensuring the ID is
+ * unique both among all those servers and existing ones
+ */
+export function ensureUniqueIds(existingServers: ServersMap, serversList: ServerData[]): ServerWithId[] {
+  const existingIds = new Set(Object.keys(existingServers));
+  const serversWithId: ServerWithId[] = [];
+
+  serversList.forEach((server) => {
+    const baseId = idForServer(server);
+
+    let id = baseId;
+    let iterations = 1;
+    while (existingIds.has(id)) {
+      id = `${baseId}-${iterations}`;
+      iterations++;
+    }
+
+    serversWithId.push({ ...server, id });
+
+    // Add this server's ID to the list, so that it is taken into consideration for the next ones
+    existingIds.add(id);
+  });
+
+  return serversWithId;
+}

src/servers/reducers/remoteServers.ts

@@ -1,11 +1,14 @@
 import type { HttpClient } from '@shlinkio/shlink-js-sdk';
 import pack from '../../../package.json';
 import { createAsyncThunk } from '../../utils/helpers/redux';
-import type { ServerData } from '../data';
 import { hasServerData } from '../data';
+import { ensureUniqueIds } from '../helpers';
 import { createServers } from './servers';
 
-const responseToServersList = (data: any): ServerData[] => (Array.isArray(data) ? data.filter(hasServerData) : []);
+const responseToServersList = (data: any) => ensureUniqueIds(
+  {},
+  (Array.isArray(data) ? data.filter(hasServerData) : []),
+);
 
 export const fetchServers = (httpClient: HttpClient) => createAsyncThunk(
   'shlink/remoteServers/fetchServers',

src/servers/reducers/servers.ts

@@ -1,7 +1,7 @@
 import type { PayloadAction } from '@reduxjs/toolkit';
 import { createSlice } from '@reduxjs/toolkit';
-import { randomUUID } from '../../utils/utils';
 import type { ServerData, ServersMap, ServerWithId } from '../data';
+import { serversListToMap } from '../helpers';
 
 interface EditServer {
   serverId: string;
@@ -15,19 +15,6 @@ interface SetAutoConnect {
 
 const initialState: ServersMap = {};
 
-const serverWithId = (server: ServerWithId | ServerData): ServerWithId => {
-  if ('id' in server) {
-    return server;
-  }
-
-  return { ...server, id: randomUUID() };
-};
-
-const serversListToMap = (servers: ServerWithId[]): ServersMap => servers.reduce<ServersMap>(
-  (acc, server) => ({ ...acc, [server.id]: server }),
-  {},
-);
-
 export const { actions, reducer } = createSlice({
   name: 'shlink/servers',
   initialState,
@@ -70,10 +57,7 @@ export const { actions, reducer } = createSlice({
       },
     },
     createServers: {
-      prepare: (servers: ServerData[]) => {
-        const payload = serversListToMap(servers.map(serverWithId));
-        return { payload };
-      },
+      prepare: (servers: ServerWithId[]) => ({ payload: serversListToMap(servers) }),
       reducer: (state, { payload: newServers }: PayloadAction<ServersMap>) => ({ ...state, ...newServers }),
     },
   },