Skip to content

Commit 0e7266d

Browse files
committed
frontend: fix loose checking for connector default override
In 0fc7832 we changed: - if (this.appliedConfig == null) + if (this.appliedConfig === null) But appliedConfig is typed as Record<string, unknown> | undefined, never null. The loose equality (undefined == null) was true, so the custom_default_value loop ran correctly. The strict equality (undefined === null) is false, so the loop was skipped for new connectors.
1 parent 7ae96ea commit 0e7266d

File tree

2 files changed

+235
-1
lines changed

2 files changed

+235
-1
lines changed
Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
/**
2+
* Copyright 2022 Redpanda Data, Inc.
3+
*
4+
* Use of this software is governed by the Business Source License
5+
* included in the file https://github.com/redpanda-data/redpanda/blob/dev/licenses/bsl.md
6+
*
7+
* As of the Change Date specified in that file, in accordance with
8+
* the Business Source License, use of this software will be governed
9+
* by the Apache License, Version 2.0
10+
*/
11+
12+
import { vi, describe, it, expect, beforeEach } from 'vitest';
13+
import type {
14+
ConnectorProperty,
15+
ConnectorValidationResult,
16+
} from '../rest-interfaces';
17+
import { DataType, PropertyImportance, PropertyWidth } from '../rest-interfaces';
18+
19+
// Mock the backend-api module before importing the store
20+
vi.mock('../backend-api', () => ({
21+
api: {
22+
validateConnectorConfig: vi.fn(),
23+
},
24+
}));
25+
26+
import { api } from '../backend-api';
27+
import { ConnectorPropertiesStore } from './state';
28+
29+
30+
function createMockProperty(overrides: {
31+
name: string;
32+
type?: string;
33+
default_value?: string | null;
34+
custom_default_value?: string;
35+
value?: string | null;
36+
required?: boolean;
37+
}): ConnectorProperty {
38+
return {
39+
definition: {
40+
name: overrides.name,
41+
type: (overrides.type ?? DataType.String) as ConnectorProperty['definition']['type'],
42+
required: overrides.required ?? false,
43+
default_value: overrides.default_value ?? null,
44+
custom_default_value: overrides.custom_default_value,
45+
importance: PropertyImportance.High,
46+
documentation: '',
47+
width: PropertyWidth.Medium,
48+
display_name: overrides.name,
49+
dependents: [],
50+
order: 0,
51+
},
52+
value: {
53+
name: overrides.name,
54+
value: overrides.value ?? null,
55+
recommended_values: [],
56+
errors: [],
57+
visible: true,
58+
},
59+
metadata: {},
60+
};
61+
}
62+
63+
function createMockValidationResult(properties: ConnectorProperty[]): ConnectorValidationResult {
64+
return {
65+
name: 'test-connector',
66+
configs: properties,
67+
steps: [
68+
{
69+
name: 'Configuration',
70+
groups: [
71+
{
72+
name: 'General',
73+
config_keys: properties.map((p) => p.definition.name),
74+
},
75+
],
76+
stepIndex: 0,
77+
},
78+
],
79+
};
80+
}
81+
82+
describe('ConnectorPropertiesStore', () => {
83+
const mockValidateConnectorConfig = vi.mocked(api.validateConnectorConfig);
84+
85+
beforeEach(() => {
86+
vi.clearAllMocks();
87+
});
88+
89+
describe('custom_default_value handling', () => {
90+
it('applies custom_default_value for new connectors (appliedConfig = undefined)', async () => {
91+
// Arrange: Property with custom_default_value
92+
const properties = [
93+
createMockProperty({
94+
name: 'flush.lsn.source',
95+
type: DataType.Boolean,
96+
default_value: null,
97+
custom_default_value: 'true',
98+
value: null,
99+
}),
100+
];
101+
102+
mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult(properties));
103+
104+
// Act: Create store with undefined appliedConfig (new connector)
105+
const store = new ConnectorPropertiesStore(
106+
'test-cluster',
107+
'io.example.ConnectorPlugin',
108+
'source',
109+
undefined // Key: undefined = new connector
110+
);
111+
112+
// Wait for async initialization
113+
await vi.waitFor(() => expect(store.initPending).toBe(false));
114+
115+
// Assert: custom_default_value should be applied
116+
const prop = store.propsByName.get('flush.lsn.source');
117+
expect(prop).toBeDefined();
118+
expect(prop?.value).toBe('true');
119+
});
120+
121+
it('preserves user config when editing existing connector (appliedConfig provided)', async () => {
122+
// Arrange: Property with custom_default_value, but user has different value
123+
const properties = [
124+
createMockProperty({
125+
name: 'flush.lsn.source',
126+
type: DataType.Boolean,
127+
default_value: null,
128+
custom_default_value: 'true',
129+
value: 'false', // User's saved value from validation
130+
}),
131+
];
132+
133+
mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult(properties));
134+
135+
// Act: Create store with appliedConfig (editing existing connector)
136+
const store = new ConnectorPropertiesStore(
137+
'test-cluster',
138+
'io.example.ConnectorPlugin',
139+
'source',
140+
{ 'flush.lsn.source': 'false' } // User's saved configuration
141+
);
142+
143+
// Wait for async initialization
144+
await vi.waitFor(() => expect(store.initPending).toBe(false));
145+
146+
// Assert: user's value should be preserved, not overwritten by custom_default_value
147+
const prop = store.propsByName.get('flush.lsn.source');
148+
expect(prop).toBeDefined();
149+
expect(prop?.value).toBe(false); // Sanitized to boolean
150+
});
151+
152+
it('applies custom_default_value only to properties that have it', async () => {
153+
// Arrange: Mix of properties with and without custom_default_value
154+
const properties = [
155+
createMockProperty({
156+
name: 'prop.with.custom.default',
157+
type: DataType.String,
158+
default_value: 'original-default',
159+
custom_default_value: 'custom-default',
160+
value: null,
161+
}),
162+
createMockProperty({
163+
name: 'prop.without.custom.default',
164+
type: DataType.String,
165+
default_value: 'original-default',
166+
// No custom_default_value
167+
value: null,
168+
}),
169+
createMockProperty({
170+
name: 'prop.with.no.defaults',
171+
type: DataType.String,
172+
default_value: null,
173+
// No custom_default_value
174+
value: null,
175+
}),
176+
];
177+
178+
mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult(properties));
179+
180+
// Act: Create store with undefined appliedConfig (new connector)
181+
const store = new ConnectorPropertiesStore(
182+
'test-cluster',
183+
'io.example.ConnectorPlugin',
184+
'source',
185+
undefined
186+
);
187+
188+
// Wait for async initialization
189+
await vi.waitFor(() => expect(store.initPending).toBe(false));
190+
191+
// Assert: Only prop with custom_default_value gets the custom default
192+
expect(store.propsByName.get('prop.with.custom.default')?.value).toBe('custom-default');
193+
expect(store.propsByName.get('prop.without.custom.default')?.value).toBe('original-default');
194+
expect(store.propsByName.get('prop.with.no.defaults')?.value).toBeNull();
195+
});
196+
197+
it('applies custom_default_value for boolean properties correctly', async () => {
198+
// Arrange: Boolean properties with various custom_default_value settings
199+
const properties = [
200+
createMockProperty({
201+
name: 'bool.custom.true',
202+
type: DataType.Boolean,
203+
default_value: null,
204+
custom_default_value: 'true',
205+
value: null,
206+
}),
207+
createMockProperty({
208+
name: 'bool.custom.false',
209+
type: DataType.Boolean,
210+
default_value: null,
211+
custom_default_value: 'false',
212+
value: null,
213+
}),
214+
];
215+
216+
mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult(properties));
217+
218+
// Act: Create store with undefined appliedConfig (new connector)
219+
const store = new ConnectorPropertiesStore(
220+
'test-cluster',
221+
'io.example.ConnectorPlugin',
222+
'source',
223+
undefined
224+
);
225+
226+
// Wait for async initialization
227+
await vi.waitFor(() => expect(store.initPending).toBe(false));
228+
229+
// Assert: custom_default_value strings should be applied
230+
expect(store.propsByName.get('bool.custom.true')?.value).toBe('true');
231+
expect(store.propsByName.get('bool.custom.false')?.value).toBe('false');
232+
});
233+
});
234+
});

frontend/src/state/connect/state.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ export class ConnectorPropertiesStore {
661661
this.propsByName.set(p.name, p);
662662
}
663663

664-
if (this.appliedConfig === null) {
664+
if (this.appliedConfig === undefined) {
665665
// Set default values
666666
for (const p of allProps) {
667667
if (p.entry.definition.custom_default_value !== undefined) {

0 commit comments

Comments
 (0)