Skip to content

Commit 6eb4613

Browse files
Rohit0301ulixius9
Rohit0301
and
ulixius9
committed
UI fails to display custom properties with dots in property name (#27390)
* UI fails to display custom properties with dots in property name * addressed PR comment * minor fix * Fix search and update issues for custom property * add unit tests * add playwright test and lint fix * minor fix * fix lint issues * added playright test --------- Co-authored-by: ulixius9 <mayursingal9@gmail.com>
1 parent 9c84629 commit 6eb4613

34 files changed

Lines changed: 578 additions & 107 deletions

‎openmetadata-service/src/main/java/org/openmetadata/service/TypeRegistry.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ public static String getCustomPropertyFQN(String entityType, String propertyName
154154
}
155155

156156
public static String getPropertyName(String propertyFQN) {
157-
return FullyQualifiedName.split(propertyFQN)[2];
157+
CustomProperty property = CUSTOM_PROPERTIES.get(propertyFQN);
158+
if (property != null) {
159+
return property.getName();
160+
}
161+
return FullyQualifiedName.unquoteName(FullyQualifiedName.split(propertyFQN)[2]);
158162
}
159163

160164
public static String getCustomPropertyType(String entityType, String propertyName) {

‎openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4377,7 +4377,7 @@ public final Object getExtension(T entity) {
43774377
}
43784378
ObjectNode objectNode = JsonUtils.getObjectNode();
43794379
for (ExtensionRecord extensionRecord : records) {
4380-
String fieldName = extensionRecord.extensionName().substring(fieldFQNPrefix.length() + 1);
4380+
String fieldName = TypeRegistry.getPropertyName(extensionRecord.extensionName());
43814381
JsonNode fieldValue = JsonUtils.readTree(extensionRecord.extensionJson());
43824382
String customPropertyType = TypeRegistry.getCustomPropertyType(entityType, fieldName);
43834383
if ("enum".equals(customPropertyType) && fieldValue.isArray() && fieldValue.size() > 1) {

‎openmetadata-service/src/test/java/org/openmetadata/service/TypeRegistryTest.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright 2021 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+
package org.openmetadata.service;
15+
16+
import static org.junit.jupiter.api.Assertions.assertEquals;
17+
18+
import org.junit.jupiter.api.AfterEach;
19+
import org.junit.jupiter.api.Test;
20+
import org.openmetadata.schema.entity.type.CustomProperty;
21+
22+
/**
23+
* Tests {@link TypeRegistry#getPropertyName(String)}.
24+
*
25+
* <p>The OpenMetadata FQN system normalises quotes — a property named {@code "/random/"} (with
26+
* literal quote characters) and a property named {@code custom.test} (with dots) end up with FQN
27+
* segments that look the same after normalisation. To return the original name to API consumers,
28+
* {@code getPropertyName} prefers the registered {@link CustomProperty#getName()} over re-deriving
29+
* from the FQN. These tests pin that behaviour.
30+
*/
31+
class TypeRegistryTest {
32+
33+
private static final String ENTITY_TYPE = "table";
34+
private static final String FQN_PREFIX_TABLE = "table.customProperties";
35+
36+
@AfterEach
37+
void cleanRegistry() {
38+
TypeRegistry.CUSTOM_PROPERTIES.clear();
39+
}
40+
41+
@Test
42+
void getPropertyName_returnsRegisteredNameWithLiteralQuotesPreserved() {
43+
// Property name has literal quote characters that the FQN system would
44+
// otherwise strip during quoteName() normalisation.
45+
String propertyName = "\"/random/\"";
46+
String fqn = TypeRegistry.getCustomPropertyFQN(ENTITY_TYPE, propertyName);
47+
TypeRegistry.CUSTOM_PROPERTIES.put(fqn, customPropertyNamed(propertyName));
48+
49+
// Sanity: FQN-level normalisation strips the literal quotes — there is no
50+
// way to recover them by parsing the FQN segment alone.
51+
assertEquals(FQN_PREFIX_TABLE + "./random/", fqn);
52+
53+
assertEquals(propertyName, TypeRegistry.getPropertyName(fqn));
54+
}
55+
56+
@Test
57+
void getPropertyName_returnsRegisteredNameForPropertyWithDots() {
58+
String propertyName = "custom.test";
59+
String fqn = TypeRegistry.getCustomPropertyFQN(ENTITY_TYPE, propertyName);
60+
TypeRegistry.CUSTOM_PROPERTIES.put(fqn, customPropertyNamed(propertyName));
61+
62+
// FQN-quoting wraps names containing dots so the FQN parser can split them.
63+
assertEquals(FQN_PREFIX_TABLE + ".\"custom.test\"", fqn);
64+
65+
// The returned name is the original (unquoted) form, not the FQN-quoted one.
66+
assertEquals(propertyName, TypeRegistry.getPropertyName(fqn));
67+
}
68+
69+
@Test
70+
void getPropertyName_returnsRegisteredNameForSimpleProperty() {
71+
String propertyName = "demo";
72+
String fqn = TypeRegistry.getCustomPropertyFQN(ENTITY_TYPE, propertyName);
73+
TypeRegistry.CUSTOM_PROPERTIES.put(fqn, customPropertyNamed(propertyName));
74+
75+
assertEquals(FQN_PREFIX_TABLE + ".demo", fqn);
76+
assertEquals(propertyName, TypeRegistry.getPropertyName(fqn));
77+
}
78+
79+
@Test
80+
void getPropertyName_fallsBackToFqnParsingWhenNotRegistered() {
81+
// Unregistered property with FQN-quoted segment (i.e., name had dots).
82+
// Without a registry hit, we must fall back to FQN parsing + unquoteName.
83+
String fqn = FQN_PREFIX_TABLE + ".\"custom.test\"";
84+
85+
assertEquals("custom.test", TypeRegistry.getPropertyName(fqn));
86+
}
87+
88+
@Test
89+
void getPropertyName_fallsBackToFqnParsingForUnquotedSegment() {
90+
String fqn = FQN_PREFIX_TABLE + ".demo";
91+
92+
assertEquals("demo", TypeRegistry.getPropertyName(fqn));
93+
}
94+
95+
@Test
96+
void getPropertyName_registeredNameWinsOverFqnFallback() {
97+
// The FQN segment is "/random/" (no quotes — they were stripped during
98+
// FQN building). The registered name has literal quotes. Without the
99+
// registry lookup, the fallback would return "/random/" and we'd lose
100+
// the original quotes — which is exactly the bug this fix addresses.
101+
String registeredName = "\"/random/\"";
102+
String fqn = FQN_PREFIX_TABLE + "./random/";
103+
TypeRegistry.CUSTOM_PROPERTIES.put(fqn, customPropertyNamed(registeredName));
104+
105+
assertEquals(registeredName, TypeRegistry.getPropertyName(fqn));
106+
}
107+
108+
private static CustomProperty customPropertyNamed(String name) {
109+
return new CustomProperty().withName(name);
110+
}
111+
}

‎openmetadata-ui/src/main/resources/ui/playwright/constant/customProperty.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -434,12 +434,5 @@ export const CUSTOM_PROPERTIES_ENTITIES = {
434434
},
435435
};
436436

437-
export const CUSTOM_PROPERTY_INVALID_NAMES = {
438-
CAPITAL_CASE: 'CapitalCase',
439-
WITH_UNDERSCORE: 'with_underscore',
440-
WITH_DOTS: 'with.',
441-
WITH_SPACE: 'with ',
442-
};
443-
444437
export const CUSTOM_PROPERTY_NAME_VALIDATION_ERROR =
445-
'Name must start with lower case with no space, underscore, or dots.';
438+
"Name must not contain '::'.";

‎openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/AdvanceSearchFilter/CustomPropertyAdvanceSeach.spec.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,122 @@ test.describe('Custom Property Advanced Search Filter for Dashboard', () => {
8282
});
8383

8484
test.describe('Text Field Custom Properties', () => {
85+
test('String CP with numeric-like string value', async ({
86+
browser,
87+
page,
88+
}) => {
89+
test.slow();
90+
const numericStringDashboard = new DashboardClass();
91+
92+
await test.step('Setup dashboard with numeric-like string value', async () => {
93+
const { apiContext, afterAction } = await createNewPage(browser);
94+
95+
await numericStringDashboard.create(apiContext);
96+
97+
await apiContext.patch(
98+
`/api/v1/dashboards/${numericStringDashboard.entityResponseData.id}`,
99+
{
100+
data: [
101+
{
102+
op: 'add',
103+
path: '/extension',
104+
value: { [propertyNames['string']]: '100' },
105+
},
106+
],
107+
headers: {
108+
'Content-Type': 'application/json-patch+json',
109+
},
110+
}
111+
);
112+
113+
await afterAction();
114+
});
115+
116+
await test.step('Equal operator finds dashboard with string value "100"', async () => {
117+
await showAdvancedSearchDialog(page);
118+
await applyCustomPropertyFilter(
119+
page,
120+
propertyNames['string'],
121+
'equal',
122+
'100'
123+
);
124+
await verifySearchResults(
125+
page,
126+
numericStringDashboard.entityResponseData.fullyQualifiedName,
127+
true,
128+
'100'
129+
);
130+
await clearAdvancedSearchFilters(page);
131+
});
132+
133+
await test.step('Not_equal operator excludes dashboard with string value "100"', async () => {
134+
await showAdvancedSearchDialog(page);
135+
await applyCustomPropertyFilter(
136+
page,
137+
propertyNames['string'],
138+
'not_equal',
139+
'100'
140+
);
141+
await verifySearchResults(
142+
page,
143+
numericStringDashboard.entityResponseData.fullyQualifiedName,
144+
false,
145+
'100'
146+
);
147+
await clearAdvancedSearchFilters(page);
148+
});
149+
150+
await test.step('Contains operator finds dashboard with partial numeric-like string "10"', async () => {
151+
await showAdvancedSearchDialog(page);
152+
await applyCustomPropertyFilter(
153+
page,
154+
propertyNames['string'],
155+
'like',
156+
'10'
157+
);
158+
await verifySearchResults(
159+
page,
160+
numericStringDashboard.entityResponseData.fullyQualifiedName,
161+
true,
162+
'10'
163+
);
164+
await clearAdvancedSearchFilters(page);
165+
});
166+
167+
await test.step('Not contains operator excludes dashboard with partial numeric-like string "10"', async () => {
168+
await showAdvancedSearchDialog(page);
169+
await applyCustomPropertyFilter(
170+
page,
171+
propertyNames['string'],
172+
'not_like',
173+
'10'
174+
);
175+
await verifySearchResults(
176+
page,
177+
numericStringDashboard.entityResponseData.fullyQualifiedName,
178+
false,
179+
'10'
180+
);
181+
await clearAdvancedSearchFilters(page);
182+
});
183+
184+
await test.step('Is not null operator finds dashboard with numeric-like string value', async () => {
185+
await showAdvancedSearchDialog(page);
186+
await applyCustomPropertyFilter(
187+
page,
188+
propertyNames['string'],
189+
'is_not_null',
190+
''
191+
);
192+
await verifySearchResults(
193+
page,
194+
numericStringDashboard.entityResponseData.fullyQualifiedName,
195+
true
196+
);
197+
await clearAdvancedSearchFilters(page);
198+
});
199+
});
200+
85201
test('String CP with all operators', async ({ page }) => {
86202
test.slow();
87203
const propertyName = propertyNames['string'];

0 commit comments

Comments
 (0)