Skip to content

Commit 2633b8d

Browse files
Fixes #25434: Preserve empty string for Kafka schemaRegistryTopicSuffixName (#27015)
* Fixes #25434: Preserve empty string for Kafka schemaRegistryTopicSuffixName * test(e2e): add Playwright coverage for empty schemaRegistryTopicSuffixName (#25434) Adds a regression test in ServiceForm.spec.ts that creates a Kafka messaging service with schemaRegistryTopicSuffixName='-value', clears the field in the edit-connection form, and asserts the PATCH payload carries an explicit empty string and that the cleared value persists on reload. Addresses review feedback on PR #27015 requesting Playwright coverage. * style: apply prettier formatting to ServiceForm.spec.ts * style: drop PR number from regression comment per Copilot review * fix(test): import COMMON_UI_SCHEMA from non-deprecated ServiceUISchema.constant per Copilot review
1 parent fafe3cf commit 2633b8d

3 files changed

Lines changed: 187 additions & 0 deletions

File tree

openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ServiceForm.spec.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { expect, test } from '@playwright/test';
1515
import * as fs from 'fs';
1616
import * as path from 'path';
1717
import { PLAYWRIGHT_INGESTION_TAG_OBJ } from '../../constant/config';
18+
import { SERVICE_TYPE } from '../../constant/service';
1819
import {
1920
CERT_FILE,
2021
lookerFormDetails,
@@ -23,9 +24,11 @@ import {
2324
supersetFormDetails3,
2425
supersetFormDetails4,
2526
} from '../../constant/serviceForm';
27+
import { MessagingServiceClass } from '../../support/entity/service/MessagingServiceClass';
2628
import { UserClass } from '../../support/user/UserClass';
2729
import { createNewPage, redirectToHomePage, uuid } from '../../utils/common';
2830
import { waitForAllLoadersToDisappear } from '../../utils/entity';
31+
import { visitServiceDetailsPage } from '../../utils/service';
2932
import { fillSupersetFormDetails } from '../../utils/serviceFormUtils';
3033

3134
const SERVICE_NAMES = {
@@ -453,5 +456,103 @@ test.describe(
453456
);
454457
});
455458
});
459+
460+
// Regression coverage for issue #25434:
461+
// Clearing `schemaRegistryTopicSuffixName` on a Kafka connection must
462+
// send an empty string to the backend instead of dropping the field,
463+
// so the cleared value is persisted on reload.
464+
test.describe('Kafka', () => {
465+
const kafkaService = new MessagingServiceClass(
466+
`pw-kafka-empty-suffix-${uuid()}`
467+
);
468+
469+
test.beforeAll(
470+
'Create Kafka service with suffix set',
471+
async ({ browser }) => {
472+
const { apiContext, afterAction } = await createNewPage(browser);
473+
await kafkaService.create(apiContext);
474+
await kafkaService.patch(apiContext, [
475+
{
476+
op: 'add',
477+
path: '/connection/config/schemaRegistryURL',
478+
value: 'http://localhost:8081',
479+
},
480+
{
481+
op: 'add',
482+
path: '/connection/config/schemaRegistryTopicSuffixName',
483+
value: '-value',
484+
},
485+
]);
486+
await afterAction();
487+
}
488+
);
489+
490+
test.afterAll('Cleanup Kafka service', async ({ browser }) => {
491+
const { apiContext, afterAction } = await createNewPage(browser);
492+
await kafkaService.delete(apiContext);
493+
await afterAction();
494+
});
495+
496+
test('should persist empty schemaRegistryTopicSuffixName when the field is cleared', async ({
497+
page,
498+
}) => {
499+
await visitServiceDetailsPage(
500+
page,
501+
{
502+
name: kafkaService.entity.name,
503+
type: SERVICE_TYPE.Messaging,
504+
},
505+
false,
506+
false
507+
);
508+
509+
await page.getByRole('tab', { name: 'Connection' }).click();
510+
await page.getByTestId('edit-connection-button').click();
511+
await waitForAllLoadersToDisappear(page);
512+
513+
const suffixInput = page.locator(
514+
String.raw`#root\/schemaRegistryTopicSuffixName`
515+
);
516+
517+
await expect(suffixInput).toHaveValue('-value');
518+
await suffixInput.clear();
519+
await expect(suffixInput).toHaveValue('');
520+
521+
const patchResponse = page.waitForResponse(
522+
(response) =>
523+
response.url().includes('/api/v1/services/messagingServices') &&
524+
response.request().method() === 'PATCH'
525+
);
526+
527+
await page.getByTestId('submit-btn').click();
528+
await page.getByTestId('submit-btn').click();
529+
530+
const patch = await patchResponse;
531+
const patchBody = patch.request().postDataJSON() as Array<{
532+
op: string;
533+
path: string;
534+
value?: unknown;
535+
}>;
536+
537+
const suffixOp = patchBody.find(
538+
(op) => op.path === '/connection/config/schemaRegistryTopicSuffixName'
539+
);
540+
541+
// The fix must send an explicit empty string, not drop the field
542+
// (which would leave the stale "-value" on the server).
543+
expect(suffixOp).toBeDefined();
544+
expect(suffixOp?.value).toBe('');
545+
546+
await waitForAllLoadersToDisappear(page);
547+
548+
// Reopen the edit form and verify the cleared value persisted.
549+
await page.getByTestId('edit-connection-button').click();
550+
await waitForAllLoadersToDisappear(page);
551+
552+
await expect(
553+
page.locator(String.raw`#root\/schemaRegistryTopicSuffixName`)
554+
).toHaveValue('');
555+
});
556+
});
456557
}
457558
);
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Copyright 2025 Collate.
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
14+
jest.mock(
15+
'../jsons/connectionSchemas/connections/messaging/kafkaConnection.json',
16+
() => ({}),
17+
{ virtual: true }
18+
);
19+
jest.mock(
20+
'../jsons/connectionSchemas/connections/messaging/redpandaConnection.json',
21+
() => ({}),
22+
{ virtual: true }
23+
);
24+
jest.mock(
25+
'../jsons/connectionSchemas/connections/messaging/customMessagingConnection.json',
26+
() => ({}),
27+
{ virtual: true }
28+
);
29+
jest.mock(
30+
'../jsons/connectionSchemas/connections/messaging/kinesisConnection.json',
31+
() => ({}),
32+
{ virtual: true }
33+
);
34+
jest.mock(
35+
'../jsons/connectionSchemas/connections/messaging/pubSubConnection.json',
36+
() => ({}),
37+
{ virtual: true }
38+
);
39+
40+
import { COMMON_UI_SCHEMA } from '../constants/ServiceUISchema.constant';
41+
import { MessagingServiceType } from '../generated/entity/services/messagingService';
42+
import { getMessagingConfig } from './MessagingServiceUtils';
43+
44+
describe('MessagingServiceUtils', () => {
45+
it('Kafka uiSchema should include ui:emptyValue for schemaRegistryTopicSuffixName', () => {
46+
const config = getMessagingConfig(MessagingServiceType.Kafka);
47+
48+
expect(config.uiSchema).toMatchObject({
49+
...COMMON_UI_SCHEMA,
50+
schemaRegistryTopicSuffixName: {
51+
'ui:emptyValue': '',
52+
},
53+
});
54+
});
55+
56+
it('Redpanda uiSchema should include ui:emptyValue for schemaRegistryTopicSuffixName', () => {
57+
const config = getMessagingConfig(MessagingServiceType.Redpanda);
58+
59+
expect(config.uiSchema).toMatchObject({
60+
...COMMON_UI_SCHEMA,
61+
schemaRegistryTopicSuffixName: {
62+
'ui:emptyValue': '',
63+
},
64+
});
65+
});
66+
67+
it('non-broker services should not include schemaRegistryTopicSuffixName uiSchema', () => {
68+
const config = getMessagingConfig(MessagingServiceType.Kinesis);
69+
70+
expect(config.uiSchema).not.toHaveProperty('schemaRegistryTopicSuffixName');
71+
});
72+
73+
it('getMessagingConfig should return only common UI schema for invalid types', () => {
74+
const config = getMessagingConfig('' as MessagingServiceType);
75+
76+
expect(config.uiSchema).toEqual({ ...COMMON_UI_SCHEMA });
77+
});
78+
});

openmetadata-ui/src/main/resources/ui/src/utils/MessagingServiceUtils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,26 @@ export const getBrokers = (config: MessagingConnection['config']) => {
3434
return isUndefined(retVal) ? '--' : retVal;
3535
};
3636

37+
const SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA = {
38+
schemaRegistryTopicSuffixName: {
39+
'ui:emptyValue': '',
40+
},
41+
};
42+
3743
export const getMessagingConfig = (type: MessagingServiceType) => {
3844
let schema = {};
3945
const uiSchema = { ...COMMON_UI_SCHEMA };
4046

4147
switch (type) {
4248
case MessagingServiceType.Kafka:
4349
schema = kafkaConnection;
50+
Object.assign(uiSchema, SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA);
4451

4552
break;
4653

4754
case MessagingServiceType.Redpanda:
4855
schema = redpandaConnection;
56+
Object.assign(uiSchema, SCHEMA_REGISTRY_SUFFIX_UI_SCHEMA);
4957

5058
break;
5159

0 commit comments

Comments
 (0)