feat: port conflict validation, autosave on Apply, extended integration tests
Port conflict validation: - api/port_registry.py: detect_conflicts() checks all service sections for shared port values - api/app.py: returns HTTP 409 on port conflict after existing range validation - webui/src/pages/Settings.jsx: JS-side detectPortConflicts() with useMemo shows inline conflict errors and blocks Save before the request is made; catch blocks surface server error messages (including 409) instead of generic fallbacks Config autosave on Apply: - webui/src/contexts/DraftConfigContext.jsx: new context; Settings registers flush callbacks per section; App calls flushAll() before applyPending() when any section is dirty - webui/src/App.jsx: wraps tree with DraftConfigProvider, handleApply shows 'saving' banner state and awaits flushAll() - webui/src/pages/Settings.jsx: registers identity + per-service flushers; propagates dirty state into context via setDirty; uses refs to avoid stale closures Extended integration test coverage (114 new tests): - tests/integration/test_config_api.py: GET/PUT config, export, import, backup lifecycle - tests/integration/test_network_services.py: DNS records + DHCP reservations CRUD - tests/integration/test_containers.py: list, restart, logs, stats; recovery polling - tests/integration/test_negative_scenarios.py: error-path coverage for all endpoints - tests/test_port_conflicts.py: 20 unit tests for port_registry.detect_conflicts() Pre-commit hook updated to skip tests/integration/ (live-stack tests require a running stack and must be run explicitly via `make test-integration`). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+22
-2
@@ -20,6 +20,7 @@ import {
|
||||
} from 'lucide-react';
|
||||
import { healthAPI, cellAPI } from './services/api';
|
||||
import { ConfigProvider } from './contexts/ConfigContext';
|
||||
import { DraftConfigProvider, useDraftConfig } from './contexts/DraftConfigContext';
|
||||
import Sidebar from './components/Sidebar';
|
||||
import Dashboard from './pages/Dashboard';
|
||||
import Peers from './pages/Peers';
|
||||
@@ -164,11 +165,21 @@ function App() {
|
||||
};
|
||||
}, [checkHealth, checkPending]);
|
||||
|
||||
const [applyStatus, setApplyStatus] = useState(null); // null | 'restarting' | 'done' | 'timeout' | 'error'
|
||||
const [applyStatus, setApplyStatus] = useState(null); // null | 'saving' | 'restarting' | 'done' | 'timeout' | 'error'
|
||||
const [applyError, setApplyError] = useState('');
|
||||
|
||||
const { flushAll, hasDirty } = useDraftConfig();
|
||||
|
||||
const handleApply = useCallback(async () => {
|
||||
setApplyError('');
|
||||
if (hasDirty()) {
|
||||
setApplyStatus('saving');
|
||||
try {
|
||||
await flushAll();
|
||||
} catch {
|
||||
// flush errors are shown via Settings toasts; continue with apply
|
||||
}
|
||||
}
|
||||
try {
|
||||
await cellAPI.applyPending();
|
||||
} catch (err) {
|
||||
@@ -197,7 +208,7 @@ function App() {
|
||||
setApplyStatus('timeout');
|
||||
setApplyError('Containers may still be starting — check docker logs if services are unavailable');
|
||||
setTimeout(() => setApplyStatus(null), 8000);
|
||||
}, []);
|
||||
}, [flushAll, hasDirty]);
|
||||
|
||||
const handleCancel = useCallback(async () => {
|
||||
await cellAPI.cancelPending();
|
||||
@@ -232,6 +243,7 @@ function App() {
|
||||
}
|
||||
|
||||
return (
|
||||
<DraftConfigProvider>
|
||||
<Router>
|
||||
<ConfigProvider>
|
||||
<div className="min-h-screen bg-gray-50">
|
||||
@@ -265,6 +277,13 @@ function App() {
|
||||
<PendingRestartBanner pending={pending} onApply={handleApply} onCancel={handleCancel} />
|
||||
)}
|
||||
|
||||
{applyStatus === 'saving' && (
|
||||
<div className="mb-6 bg-blue-50 border border-blue-200 rounded-lg p-4 flex items-center gap-3">
|
||||
<RefreshCw className="h-5 w-5 text-blue-500 animate-spin flex-shrink-0" />
|
||||
<span className="text-sm font-medium text-blue-800">Saving settings…</span>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{applyStatus === 'restarting' && (
|
||||
<div className="mb-6 bg-blue-50 border border-blue-200 rounded-lg p-4 flex items-center gap-3">
|
||||
<RefreshCw className="h-5 w-5 text-blue-500 animate-spin flex-shrink-0" />
|
||||
@@ -307,6 +326,7 @@ function App() {
|
||||
</div>
|
||||
</ConfigProvider>
|
||||
</Router>
|
||||
</DraftConfigProvider>
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import { createContext, useContext, useRef, useCallback } from 'react';
|
||||
|
||||
const DraftConfigContext = createContext(null);
|
||||
|
||||
export function DraftConfigProvider({ children }) {
|
||||
const flushersRef = useRef({}); // key → async flush fn
|
||||
|
||||
const registerFlusher = useCallback((key, fn) => {
|
||||
flushersRef.current[key] = fn;
|
||||
return () => { delete flushersRef.current[key]; }; // cleanup
|
||||
}, []);
|
||||
|
||||
const hasDirtyRef = useRef({}); // key → boolean
|
||||
|
||||
const setDirty = useCallback((key, isDirty) => {
|
||||
hasDirtyRef.current[key] = isDirty;
|
||||
}, []);
|
||||
|
||||
const hasDirty = useCallback(() => {
|
||||
return Object.values(hasDirtyRef.current).some(Boolean);
|
||||
}, []);
|
||||
|
||||
const flushAll = useCallback(async () => {
|
||||
const flushers = Object.values(flushersRef.current);
|
||||
await Promise.all(flushers.map(fn => fn()));
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<DraftConfigContext.Provider value={{ registerFlusher, setDirty, hasDirty, flushAll }}>
|
||||
{children}
|
||||
</DraftConfigContext.Provider>
|
||||
);
|
||||
}
|
||||
|
||||
export function useDraftConfig() {
|
||||
return useContext(DraftConfigContext);
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useState, useEffect, useCallback } from 'react';
|
||||
import { useState, useEffect, useCallback, useRef, useMemo } from 'react';
|
||||
import { useConfig } from '../contexts/ConfigContext';
|
||||
import { useDraftConfig } from '../contexts/DraftConfigContext';
|
||||
import {
|
||||
Settings as SettingsIcon, Server, Shield, Network, Mail, Calendar,
|
||||
HardDrive, GitBranch, Archive, Upload, Download, Trash2, RotateCcw,
|
||||
@@ -76,6 +77,38 @@ function isValidPort(v) {
|
||||
return Number.isInteger(n) && n >= 1 && n <= 65535;
|
||||
}
|
||||
|
||||
// Mirror of api/port_registry.py PORT_FIELDS — must stay in sync
|
||||
const PORT_CONFLICT_FIELDS = {
|
||||
network: ['dns_port'],
|
||||
wireguard: ['port'],
|
||||
email: ['smtp_port', 'submission_port', 'imap_port', 'webmail_port'],
|
||||
calendar: ['port'],
|
||||
files: ['port', 'manager_port'],
|
||||
};
|
||||
|
||||
function detectPortConflicts(configs) {
|
||||
const portMap = {};
|
||||
for (const [section, fields] of Object.entries(PORT_CONFLICT_FIELDS)) {
|
||||
const sec = configs[section] || {};
|
||||
for (const field of fields) {
|
||||
const raw = sec[field];
|
||||
if (raw === undefined || raw === null || raw === '') continue;
|
||||
const n = parseInt(raw, 10);
|
||||
if (isNaN(n)) continue;
|
||||
(portMap[n] = portMap[n] || []).push([section, field]);
|
||||
}
|
||||
}
|
||||
const result = {};
|
||||
for (const [port, slots] of Object.entries(portMap)) {
|
||||
if (slots.length < 2) continue;
|
||||
const others = slots.map(([s, f]) => `${s}.${f}`).join(', ');
|
||||
for (const [section, field] of slots) {
|
||||
result[`${section}|${field}`] = `Port ${port} conflicts with ${others}`;
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
function isValidIp(v) {
|
||||
if (!v || !v.trim()) return false;
|
||||
const m = v.trim().match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/);
|
||||
@@ -364,6 +397,7 @@ const SERVICE_DEFS = [
|
||||
function Settings() {
|
||||
const toasts = useToasts();
|
||||
const { refresh: refreshConfig } = useConfig();
|
||||
const draftConfig = useDraftConfig();
|
||||
|
||||
// identity
|
||||
const [identity, setIdentity] = useState({ cell_name: '', domain: '', ip_range: '' });
|
||||
@@ -375,6 +409,8 @@ function Settings() {
|
||||
const [serviceDirty, setServiceDirty] = useState({});
|
||||
const [serviceSaving, setServiceSaving] = useState({});
|
||||
|
||||
const portConflicts = useMemo(() => detectPortConflicts(serviceConfigs), [serviceConfigs]);
|
||||
|
||||
// backups
|
||||
const [backups, setBackups] = useState([]);
|
||||
const [backupsLoading, setBackupsLoading] = useState(false);
|
||||
@@ -427,10 +463,11 @@ function Settings() {
|
||||
try {
|
||||
const res = await cellAPI.updateConfig(identity);
|
||||
setIdentityDirty(false);
|
||||
draftConfig?.setDirty('identity', false);
|
||||
_applyResult(res, 'Cell identity');
|
||||
refreshConfig();
|
||||
} catch {
|
||||
toast('Failed to save identity', 'error');
|
||||
} catch (err) {
|
||||
toast(err.response?.data?.error || 'Failed to save identity', 'error');
|
||||
} finally {
|
||||
setIdentitySaving(false);
|
||||
}
|
||||
@@ -440,15 +477,18 @@ function Settings() {
|
||||
const saveService = async (key) => {
|
||||
const { defaults } = SERVICE_DEFS.find((d) => d.key === key) || {};
|
||||
const data = { ...(defaults || {}), ...(serviceConfigs[key] || {}) };
|
||||
if (Object.keys(validateServiceConfig(key, data)).length > 0) return;
|
||||
const hasFieldErrors = Object.keys(validateServiceConfig(key, data)).length > 0;
|
||||
const hasConflicts = (PORT_CONFLICT_FIELDS[key] || []).some(f => portConflicts[`${key}|${f}`]);
|
||||
if (hasFieldErrors || hasConflicts) return;
|
||||
setServiceSaving((s) => ({ ...s, [key]: true }));
|
||||
try {
|
||||
const res = await cellAPI.updateConfig({ [key]: serviceConfigs[key] });
|
||||
setServiceDirty((d) => ({ ...d, [key]: false }));
|
||||
draftConfig?.setDirty(key, false);
|
||||
_applyResult(res, key);
|
||||
refreshConfig();
|
||||
} catch {
|
||||
toast(`Failed to save ${key} config`, 'error');
|
||||
} catch (err) {
|
||||
toast(err.response?.data?.error || `Failed to save ${key} config`, 'error');
|
||||
} finally {
|
||||
setServiceSaving((s) => ({ ...s, [key]: false }));
|
||||
}
|
||||
@@ -457,8 +497,42 @@ function Settings() {
|
||||
const updateServiceConfig = (key, data) => {
|
||||
setServiceConfigs((prev) => ({ ...prev, [key]: data }));
|
||||
setServiceDirty((d) => ({ ...d, [key]: true }));
|
||||
draftConfig?.setDirty(key, true);
|
||||
};
|
||||
|
||||
// ── Flusher registration (autosave on Apply) ──────────────────────────────
|
||||
// Use refs so flush functions always see current dirty/save state without stale closures.
|
||||
const identityDirtyRef = useRef(identityDirty);
|
||||
useEffect(() => { identityDirtyRef.current = identityDirty; }, [identityDirty]);
|
||||
|
||||
const serviceDirtyRef = useRef(serviceDirty);
|
||||
useEffect(() => { serviceDirtyRef.current = serviceDirty; }, [serviceDirty]);
|
||||
|
||||
const saveIdentityRef = useRef(saveIdentity);
|
||||
useEffect(() => { saveIdentityRef.current = saveIdentity; }, [saveIdentity]);
|
||||
|
||||
const saveServiceRef = useRef(saveService);
|
||||
useEffect(() => { saveServiceRef.current = saveService; }, [saveService]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!draftConfig) return;
|
||||
const unregister = draftConfig.registerFlusher('identity', async () => {
|
||||
if (identityDirtyRef.current) await saveIdentityRef.current();
|
||||
});
|
||||
return unregister;
|
||||
}, [draftConfig]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!draftConfig) return;
|
||||
const unregisters = SERVICE_DEFS.map(({ key }) =>
|
||||
draftConfig.registerFlusher(key, async () => {
|
||||
if (serviceDirtyRef.current[key]) await saveServiceRef.current(key);
|
||||
})
|
||||
);
|
||||
return () => unregisters.forEach((fn) => fn());
|
||||
}, [draftConfig]);
|
||||
// ─────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// backups
|
||||
const createBackup = async () => {
|
||||
setBackupCreating(true);
|
||||
@@ -551,21 +625,21 @@ function Settings() {
|
||||
<Field label="Cell Name">
|
||||
<TextInput
|
||||
value={identity.cell_name}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, cell_name: v })); setIdentityDirty(true); }}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, cell_name: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }}
|
||||
placeholder="mycell"
|
||||
/>
|
||||
</Field>
|
||||
<Field label="Domain">
|
||||
<TextInput
|
||||
value={identity.domain}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, domain: v })); setIdentityDirty(true); }}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, domain: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }}
|
||||
placeholder="cell.local"
|
||||
/>
|
||||
</Field>
|
||||
<Field label="IP Range" hint="Docker bridge subnet" error={ipRangeError}>
|
||||
<TextInput
|
||||
value={identity.ip_range}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, ip_range: v })); setIdentityDirty(true); }}
|
||||
onChange={(v) => { setIdentity((i) => ({ ...i, ip_range: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }}
|
||||
placeholder="172.20.0.0/16"
|
||||
/>
|
||||
</Field>
|
||||
@@ -592,7 +666,12 @@ function Settings() {
|
||||
</div>
|
||||
{SERVICE_DEFS.map(({ key, label, icon: Icon, Form, defaults }) => {
|
||||
const data = { ...defaults, ...(serviceConfigs[key] || {}) };
|
||||
const errors = validateServiceConfig(key, data);
|
||||
const conflictErrors = {};
|
||||
for (const field of (PORT_CONFLICT_FIELDS[key] || [])) {
|
||||
const msg = portConflicts[`${key}|${field}`];
|
||||
if (msg) conflictErrors[field] = msg;
|
||||
}
|
||||
const errors = { ...validateServiceConfig(key, data), ...conflictErrors };
|
||||
const hasErrors = Object.keys(errors).length > 0;
|
||||
const dirty = serviceDirty[key];
|
||||
const saving = serviceSaving[key];
|
||||
|
||||
Reference in New Issue
Block a user