fix: Strip trailing slash for connections (#752)

This commit is contained in:
Tom Alexander 2025-04-29 13:38:21 -04:00 committed by GitHub
parent 9a9581b658
commit 7de8916074
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 659 additions and 430 deletions

View file

@ -0,0 +1,5 @@
---
"@hyperdx/app": patch
---
Removes trailing slash for connection urls

View file

@ -8,6 +8,7 @@ import {
formatDate,
formatNumber,
getMetricTableName,
stripTrailingSlash,
useQueryHistory,
} from '../utils';
@ -473,6 +474,48 @@ describe('useLocalStorage', () => {
});
});
describe('stripTrailingSlash', () => {
it('should throw an error for nullish values', () => {
expect(() => stripTrailingSlash(null)).toThrow(
'URL must be a non-empty string',
);
expect(() => stripTrailingSlash(undefined)).toThrow(
'URL must be a non-empty string',
);
});
it('should throw an error for non-string values', () => {
expect(() => stripTrailingSlash(123 as any)).toThrow(
'URL must be a non-empty string',
);
expect(() => stripTrailingSlash({} as any)).toThrow(
'URL must be a non-empty string',
);
});
it('should remove trailing slash from URLs', () => {
expect(stripTrailingSlash('http://example.com/')).toBe(
'http://example.com',
);
expect(stripTrailingSlash('http://example.com/api/')).toBe(
'http://example.com/api',
);
});
it('should not modify URLs without trailing slash', () => {
expect(stripTrailingSlash('http://example.com')).toBe('http://example.com');
expect(stripTrailingSlash('http://example.com/api')).toBe(
'http://example.com/api',
);
});
it('should handle URLs with multiple trailing slashes', () => {
expect(stripTrailingSlash('http://example.com///')).toBe(
'http://example.com//',
);
});
});
describe('useQueryHistory', () => {
const mockGetItem = jest.fn();
const mockSetItem = jest.fn();

View file

@ -13,6 +13,7 @@ import {
useDeleteConnection,
useUpdateConnection,
} from '@/connection';
import { stripTrailingSlash } from '@/utils';
import ConfirmDeleteMenu from './ConfirmDeleteMenu';
@ -32,9 +33,10 @@ function useTestConnection({
useState<TestConnectionState | null>(null);
const handleTestConnection = useCallback(async () => {
const host = getValues('host');
const hostValue = getValues('host');
const username = getValues('username');
const password = getValues('password');
const host = stripTrailingSlash(hostValue);
if (testConnectionState) {
return;
@ -134,9 +136,15 @@ export function ConnectionForm({
const deleteConnection = useDeleteConnection();
const onSubmit = (data: Connection) => {
// Make sure we don't save a trailing slash in the host
const normalizedData = {
...data,
host: stripTrailingSlash(data.host),
};
if (isNew) {
createConnection.mutate(
{ connection: data },
{ connection: normalizedData },
{
onSuccess: () => {
notifications.show({
@ -157,7 +165,7 @@ export function ConnectionForm({
);
} else {
updateConnection.mutate(
{ connection: data, id: connection.id },
{ connection: normalizedData, id: connection.id },
{
onSuccess: () => {
notifications.show({

View file

@ -0,0 +1,165 @@
import React from 'react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { Connection } from '../../connection';
import { ConnectionForm } from '../ConnectionForm';
import '@testing-library/jest-dom';
// --- Mocks ---
const mockCreateMutate = jest.fn();
const mockUpdateMutate = jest.fn();
jest.mock('@/connection', () => ({
...jest.requireActual('@/connection'),
useCreateConnection: () => ({
mutate: mockCreateMutate,
isPending: false,
}),
useUpdateConnection: () => ({
mutate: mockUpdateMutate,
isPending: false,
}),
useDeleteConnection: () => ({
mutate: jest.fn(),
isPending: false,
}),
}));
jest.mock('@mantine/notifications', () => ({
notifications: {
show: jest.fn(),
},
}));
const mockTestConnectionMutateAsync = jest.fn();
jest.mock('@/api', () => ({
...(jest.requireActual('@/api') ?? {}),
useTestConnection: () => ({
mutateAsync: mockTestConnectionMutateAsync.mockResolvedValue({
success: true,
}),
}),
}));
// --- Test Suite ---
describe('ConnectionForm', () => {
const baseConnection: Connection = {
id: '',
name: 'Test Connection',
host: 'http://localhost:8123',
username: 'default',
password: '',
};
beforeEach(() => {
mockCreateMutate.mockClear();
mockUpdateMutate.mockClear();
mockTestConnectionMutateAsync.mockClear();
(
jest.requireMock('@mantine/notifications') as any
).notifications.show.mockClear();
});
it('should save connection with trailing slash removed from host when creating', async () => {
renderWithMantine(
<ConnectionForm connection={baseConnection} isNew={true} />,
);
const hostInput = screen.getByPlaceholderText('http://localhost:8123');
const nameInput = screen.getByPlaceholderText('My Clickhouse Server');
const submitButton = screen.getByRole('button', { name: 'Create' });
await fireEvent.change(nameInput, { target: { value: 'Test Name' } });
await fireEvent.change(hostInput, {
target: { value: 'http://example.com:8123/' },
}); // Host with trailing slash
fireEvent.click(submitButton);
await waitFor(() => {
expect(mockCreateMutate).toHaveBeenCalledTimes(1);
});
// Check the arguments passed to the mutate function
expect(mockCreateMutate).toHaveBeenCalledWith(
expect.objectContaining({
connection: expect.objectContaining({
host: 'http://example.com:8123',
name: 'Test Name',
}),
}),
expect.anything(),
);
});
it('should save connection with trailing slash removed from host when updating', async () => {
const existingConnection = {
...baseConnection,
id: 'existing-id',
host: 'http://old.com/',
};
renderWithMantine(
<ConnectionForm connection={existingConnection} isNew={false} />,
);
const hostInput = screen.getByPlaceholderText('http://localhost:8123');
const submitButton = screen.getByRole('button', { name: 'Save' });
// Update host
await fireEvent.change(hostInput, {
target: { value: 'http://updated.com:8123/' },
});
fireEvent.click(submitButton);
// Wait for mutate to be called and assert
await waitFor(() => {
expect(mockUpdateMutate).toHaveBeenCalledTimes(1);
});
// Check the arguments passed to the mutate function
expect(mockUpdateMutate).toHaveBeenCalledWith(
expect.objectContaining({
id: 'existing-id',
connection: expect.objectContaining({
host: 'http://updated.com:8123',
}),
}),
expect.anything(),
);
});
it('should use stripped host for test connection', async () => {
renderWithMantine(
<ConnectionForm connection={baseConnection} isNew={true} />,
);
const hostInput = screen.getByPlaceholderText('http://localhost:8123');
const nameInput = screen.getByPlaceholderText('My Clickhouse Server');
const testButton = screen.getByRole('button', { name: 'Test Connection' });
await fireEvent.change(nameInput, { target: { value: 'Test Name' } });
await fireEvent.change(hostInput, {
target: { value: 'http://test.com:8123/' },
});
// Ensure form state is valid before clicking test
await waitFor(() => expect(testButton).not.toBeDisabled());
fireEvent.click(testButton);
await waitFor(() =>
expect(mockTestConnectionMutateAsync).toHaveBeenCalled(),
);
// Assert that the mock API call received the stripped host
expect(mockTestConnectionMutateAsync).toHaveBeenCalledTimes(1);
expect(mockTestConnectionMutateAsync).toHaveBeenCalledWith(
expect.objectContaining({
host: 'http://test.com:8123',
}),
);
});
});

View file

@ -747,3 +747,11 @@ export function getMetricTableName(
export function toArray<T>(obj?: T | T[]): T[] {
return !obj ? [] : Array.isArray(obj) ? obj : [obj];
}
// Helper function to remove trailing slash
export const stripTrailingSlash = (url: string | undefined | null): string => {
if (!url || typeof url !== 'string') {
throw new Error('URL must be a non-empty string');
}
return url.endsWith('/') ? url.slice(0, -1) : url;
};

View file

@ -1,53 +1,53 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
},
{
"key": "test-id",
"value": {
"stringValue": "default"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
},
{
"key": "test-id",
"value": {
"stringValue": "default"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "[note] this is very much not JSON even though it starts with an array char"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "[note] this is very much not JSON even though it starts with an array char"
}
},
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "{note} this is very much not JSON even though it starts with an object char"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "NOTE: this is very much not JSON"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "this has some {Key {Value { '{' } } invalid JSON in it"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "{note} this is very much not JSON even though it starts with an object char"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "NOTE: this is very much not JSON"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "this has some {Key {Value { '{' } } invalid JSON in it"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,67 +1,67 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
},
{
"key": "test-id",
"value": {
"stringValue": "json-string"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
},
{
"key": "test-id",
"value": {
"stringValue": "json-string"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "{\"attr\":{\"intValue\": 1},\"found\":false,\"message\":\"this should be parsed into a map\"}"
}
},
"scopeLogs": [
{
"timeUnixNano": "1901999580000000001",
"attributes": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "{\"attr\":{\"intValue\": 1},\"found\":false,\"message\":\"this should be parsed into a map\"}"
}
},
{
"timeUnixNano": "1901999580000000001",
"attributes": [
{
"key": "userAttr",
"value": {
"boolValue": true
}
}
],
"body": {
"stringValue": "{\"bodyAttr\":12345,\"message\":\"this has an existing user attribute that should be preserved.\"}"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "should find the trailing JSON object {\"found\":true,\"position\":\"trailing\"}"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "{\"found\":true,\"position\":\"leading\"} should find the leading JSON object "
}
},
{
"timeUnixNano": "1901999580000000004",
"body": {
"stringValue": "should find a wrapped JSON object {\"found\":true,\"position\":\"wrapped\"} between text"
}
}
]
"key": "userAttr",
"value": {
"boolValue": true
}
}
]
],
"body": {
"stringValue": "{\"bodyAttr\":12345,\"message\":\"this has an existing user attribute that should be preserved.\"}"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "should find the trailing JSON object {\"found\":true,\"position\":\"trailing\"}"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "{\"found\":true,\"position\":\"leading\"} should find the leading JSON object "
}
},
{
"timeUnixNano": "1901999580000000004",
"body": {
"stringValue": "should find a wrapped JSON object {\"found\":true,\"position\":\"wrapped\"} between text"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,56 +1,56 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
},
{
"key": "test-id",
"value": {
"stringValue": "otel-map"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"kvlistValue": {
"values": [
{
"key": "suite-id",
"value": {
"stringValue": "auto-parse"
}
"key": "message",
"value": {
"stringValue": "data sent as OTEL map should also extend the log attributes"
}
},
{
"key": "test-id",
"value": {
"stringValue": "otel-map"
}
"key": "user-id",
"value": {
"stringValue": "1234"
}
},
{
"key": "account-id",
"value": {
"stringValue": "550e8400-e29b-41d4-a716-446655440000"
}
}
]
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"kvlistValue": {
"values": [
{
"key": "message",
"value": {
"stringValue": "data sent as OTEL map should also extend the log attributes"
}
},
{
"key": "user-id",
"value": {
"stringValue": "1234"
}
},
{
"key": "account-id",
"value": {
"stringValue": "550e8400-e29b-41d4-a716-446655440000"
}
}
]
}
}
}
]
]
}
]
}
}
]
}
]
}
]
}
]
}

View file

@ -1,41 +1,41 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-debug"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-debug"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000008",
"body": {
"stringValue": "debug - checkpoint a"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000008",
"body": {
"stringValue": "debug - checkpoint a"
}
},
{
"timeUnixNano": "1901999580000000009",
"body": {
"stringValue": "dbug: value=1,found=false"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000009",
"body": {
"stringValue": "dbug: value=1,found=false"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,41 +1,41 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-error"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-error"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000004",
"body": {
"stringValue": "error: things aren't good"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000004",
"body": {
"stringValue": "error: things aren't good"
}
},
{
"timeUnixNano": "1901999580000000005",
"body": {
"stringValue": "err 00019: something went wrong"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000005",
"body": {
"stringValue": "err 00019: something went wrong"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,53 +1,53 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-fatal"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-fatal"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "ALERT: something is wrong here"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "ALERT: something is wrong here"
}
},
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "[thread-123] critical: disk issue detected"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "Emerg: action required"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "2025-05-03T03:23:14Z [thread-1234] c.b.e.b.e.AbstractDatabaseConnectionAdapterFactor [FATAL] this error is a huge problem"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "[thread-123] critical: disk issue detected"
}
},
{
"timeUnixNano": "1901999580000000002",
"body": {
"stringValue": "Emerg: action required"
}
},
{
"timeUnixNano": "1901999580000000003",
"body": {
"stringValue": "2025-05-03T03:23:14Z [thread-1234] c.b.e.b.e.AbstractDatabaseConnectionAdapterFactor [FATAL] this error is a huge problem"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,41 +1,41 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-info"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-info"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "default behavior if nothing is specified"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"body": {
"stringValue": "default behavior if nothing is specified"
}
},
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "2025-05-03T03:23:14Z [thread-1234] c.b.e.b.e.AbstractDatabaseConnectionAdapterFactor - request_id= the rules should only infer a logging level if the substring is within the first 255 characters so that stuff later in the message doesn't trigger the inference behavior like FATAL or ERROR"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000001",
"body": {
"stringValue": "2025-05-03T03:23:14Z [thread-1234] c.b.e.b.e.AbstractDatabaseConnectionAdapterFactor - request_id= the rules should only infer a logging level if the substring is within the first 255 characters so that stuff later in the message doesn't trigger the inference behavior like FATAL or ERROR"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,35 +1,35 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-trace"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-trace"
}
}
]
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000010",
"body": {
"stringValue": "this is a trace through logic"
}
}
]
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000010",
"body": {
"stringValue": "this is a trace through logic"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,41 +1,41 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-warn"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "infer-warn"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000006",
"body": {
"stringValue": "app=1, notice: that things are not going well"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000006",
"body": {
"stringValue": "app=1, notice: that things are not going well"
}
},
{
"timeUnixNano": "1901999580000000007",
"body": {
"stringValue": "warn: something something"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000007",
"body": {
"stringValue": "warn: something something"
}
}
]
}
]
}
]
}
]
}

View file

@ -1,44 +1,44 @@
{
"resourceLogs": [
"resourceLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "skip-infer"
}
}
]
},
"scopeLogs": [
{
"resource": {
"attributes": [
{
"key": "suite-id",
"value": {
"stringValue": "hdx-1514"
}
},
{
"key": "test-id",
"value": {
"stringValue": "skip-infer"
}
}
]
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"severity_number": 0,
"severity_text": "debug",
"body": {
"stringValue": "2025-05-03T03:23:14Z [INFO] this should skip because the severity_text is already set"
}
},
"scopeLogs": [
{
"scope": {},
"logRecords": [
{
"timeUnixNano": "1901999580000000000",
"severity_number": 0,
"severity_text": "debug",
"body": {
"stringValue": "2025-05-03T03:23:14Z [INFO] this should skip because the severity_text is already set"
}
},
{
"timeUnixNano": "1901999580000000001",
"severity_number": 9,
"body": {
"stringValue": "[warn] this should also be skipped because a severity_number was set even if it's missing text"
}
}
]
}
]
{
"timeUnixNano": "1901999580000000001",
"severity_number": 9,
"body": {
"stringValue": "[warn] this should also be skipped because a severity_number was set even if it's missing text"
}
}
]
}
]
}
]
}
]
}