Skip to content

Commit cb1e53a

Browse files
aditya-gupta36aditya-gupta36
authored
ATLAS-5053: Add recursive validation in Basic Search API to fix regression issue (#373)
1 parent b45b496 commit cb1e53a

File tree

7 files changed

+583
-142
lines changed

7 files changed

+583
-142
lines changed

‎webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -891,28 +891,43 @@ private void validateSearchParameters(SearchParameters parameters) throws AtlasB
891891
}
892892

893893
private void validateEntityFilter(SearchParameters parameters) throws AtlasBaseException {
894-
FilterCriteria entityFilter = parameters.getEntityFilters();
894+
validateNestedCriteria(parameters.getEntityFilters());
895+
validateNestedCriteria(parameters.getTagFilters());
896+
}
897+
898+
private void validateNestedCriteria(SearchParameters.FilterCriteria criteria) throws AtlasBaseException {
899+
if (criteria == null) {
900+
return; // Nothing to validate
901+
}
895902

896-
if (entityFilter == null) {
903+
boolean hasComposite = criteria.getCriterion() != null && !criteria.getCriterion().isEmpty();
904+
boolean hasLeaf = StringUtils.isNotEmpty(criteria.getAttributeName())
905+
|| criteria.getOperator() != null
906+
|| StringUtils.isNotEmpty(criteria.getAttributeValue());
907+
908+
if (!hasComposite && !hasLeaf) {
909+
// It's an empty filter object — skip (backward compatibility)
897910
return;
898911
}
899912

900-
if (entityFilter.getCriterion() != null &&
901-
!entityFilter.getCriterion().isEmpty()) {
902-
if (entityFilter.getCondition() == null || StringUtils.isEmpty(entityFilter.getCondition().toString())) {
913+
if (hasComposite) {
914+
if (criteria.getCondition() == null || StringUtils.isEmpty(criteria.getCondition().toString())) {
903915
throw new AtlasBaseException("Condition (AND/OR) must be specified when using multiple filters.");
904916
}
905917

906-
for (FilterCriteria filterCriteria : entityFilter.getCriterion()) {
907-
validateCriteria(filterCriteria);
918+
for (FilterCriteria filterCriteria : criteria.getCriterion()) {
919+
if (filterCriteria != null) {
920+
validateNestedCriteria(filterCriteria); // Recursive check
921+
}
908922
}
909923
}
910-
else {
911-
validateCriteria(entityFilter);
924+
925+
if (hasLeaf) {
926+
validateLeafFilterCriteria(criteria);
912927
}
913928
}
914929

915-
private void validateCriteria(SearchParameters.FilterCriteria criteria) throws AtlasBaseException {
930+
private void validateLeafFilterCriteria(SearchParameters.FilterCriteria criteria) throws AtlasBaseException {
916931
if (criteria.getOperator() == null) {
917932
throw new AtlasBaseException(AtlasErrorCode.INVALID_OPERATOR, criteria.getAttributeName());
918933
}

‎webapp/src/test/java/org/apache/atlas/web/integration/BasicSearchIT.java‎

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.atlas.model.typedef.AtlasTypesDef;
3636
import org.apache.atlas.type.AtlasTypeUtil;
3737
import org.apache.atlas.utils.TestResourceFileUtils;
38+
import org.apache.commons.lang.StringUtils;
3839
import org.testng.annotations.BeforeClass;
3940
import org.testng.annotations.DataProvider;
4041
import org.testng.annotations.Test;
@@ -46,7 +47,6 @@
4647
import java.util.Collections;
4748
import java.util.HashMap;
4849
import java.util.List;
49-
import java.util.function.Predicate;
5050

5151
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
5252
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.PUBLIC_ONLY;
@@ -228,21 +228,33 @@ public void testSavedSearch(String jsonFile) {
228228
}
229229

230230
@Test
231-
public void testAttributeSearchInvalidOperator() {
232-
runNegativeSearchTest("search-parameters/operator", "ATLAS-400-00-103", parameters -> parameters.getEntityFilters() != null && parameters.getEntityFilters().getOperator() != null);
231+
public void testComplexFilterInvalidOperator() {
232+
runNegativeSearchTestForBasicSearch("search-parameters/operator", "ATLAS-400-00-103",
233+
parameters -> (parameters.getEntityFilters() != null && containsAnyOperator(parameters.getEntityFilters())) ||
234+
(parameters.getTagFilters() != null && containsAnyOperator(parameters.getTagFilters())));
235+
runNegativeSearchTestForQuickSearch("search-parameters/operator", "ATLAS-400-00-103",
236+
parameters -> parameters.getEntityFilters() != null && containsAnyOperator(parameters.getEntityFilters()));
233237
}
234238

235239
@Test
236-
public void testAttributeSearchEmptyNameAttribute() {
237-
runNegativeSearchTest("search-parameters/attribute-name", "ATLAS-400-00-104", parameters -> parameters.getEntityFilters() != null && parameters.getEntityFilters().getAttributeName() != null);
240+
public void testComplexFilterEmptyAttributeName() {
241+
runNegativeSearchTestForBasicSearch("search-parameters/attribute-name", "ATLAS-400-00-104",
242+
parameters -> (parameters.getEntityFilters() != null && containsAnyAttributeName(parameters.getEntityFilters())) ||
243+
(parameters.getTagFilters() != null && containsAnyAttributeName(parameters.getTagFilters())));
244+
runNegativeSearchTestForQuickSearch("search-parameters/attribute-name", "ATLAS-400-00-104",
245+
parameters -> parameters.getEntityFilters() != null && containsAnyAttributeName(parameters.getEntityFilters()));
238246
}
239247

240248
@Test
241-
public void testAttributeSearchEmptyValueAttribute() {
242-
runNegativeSearchTest("search-parameters/attribute-value", "ATLAS-400-00-105", parameters -> parameters.getEntityFilters() != null && parameters.getEntityFilters().getAttributeValue() != null);
249+
public void testComplexFilterEmptyAttributeValue() {
250+
runNegativeSearchTestForBasicSearch("search-parameters/attribute-value", "ATLAS-400-00-105",
251+
parameters -> (parameters.getEntityFilters() != null && containsAnyAttributeValue(parameters.getEntityFilters())) ||
252+
(parameters.getTagFilters() != null && containsAnyAttributeValue(parameters.getTagFilters())));
253+
runNegativeSearchTestForQuickSearch("search-parameters/attribute-value", "ATLAS-400-00-105",
254+
parameters -> parameters.getEntityFilters() != null && containsAnyAttributeValue(parameters.getEntityFilters()));
243255
}
244256

245-
public void runNegativeSearchTest(String jsonFile, String expectedErrorCode, java.util.function.Predicate<SearchParameters> paramFilter) {
257+
public void runNegativeSearchTestForBasicSearch(String jsonFile, String expectedErrorCode, java.util.function.Predicate<SearchParameters> paramFilter) {
246258
try {
247259
BasicSearchParametersWithExpectation[] testExpectations = TestResourceFileUtils.readObjectFromJson(jsonFile, BasicSearchParametersWithExpectation[].class);
248260
assertNotNull(testExpectations);
@@ -264,6 +276,66 @@ public void runNegativeSearchTest(String jsonFile, String expectedErrorCode, jav
264276
}
265277
}
266278

279+
public void runNegativeSearchTestForQuickSearch(String jsonFile, String expectedErrorCode, java.util.function.Predicate<QuickSearchParameters> qspParamFilter) {
280+
try {
281+
BasicSearchParametersWithExpectation[] testExpectations = TestResourceFileUtils.readObjectFromJson(jsonFile, BasicSearchParametersWithExpectation[].class);
282+
assertNotNull(testExpectations);
283+
Arrays
284+
.stream(testExpectations)
285+
.map(testExpectation -> testExpectation.getSearchParameters())
286+
.map(sp -> {
287+
QuickSearchParameters qsp = new QuickSearchParameters();
288+
qsp.setTypeName(sp.getTypeName());
289+
qsp.setEntityFilters(sp.getEntityFilters());
290+
return qsp; })
291+
.filter(qspParamFilter)
292+
.forEach(params -> {
293+
try {
294+
atlasClientV2.quickSearch(params);
295+
}
296+
catch (AtlasServiceException e) {
297+
assertTrue(e.getMessage().contains(expectedErrorCode),
298+
"Expected error code " + expectedErrorCode + " in exception message: " + e.getMessage());
299+
}
300+
});
301+
} catch (IOException e) {
302+
fail(e.getMessage());
303+
}
304+
}
305+
306+
private boolean containsAnyMatchingCriteria(SearchParameters.FilterCriteria filterCriteria, java.util.function.Predicate<SearchParameters.FilterCriteria> matcher) {
307+
if (filterCriteria == null) {
308+
return false;
309+
}
310+
if (matcher.test(filterCriteria)) {
311+
return true;
312+
}
313+
if (filterCriteria.getCriterion() != null) {
314+
return filterCriteria.getCriterion().stream()
315+
.filter(fc -> fc != null)
316+
.anyMatch(fc -> containsAnyMatchingCriteria(fc, matcher));
317+
}
318+
319+
return false;
320+
}
321+
322+
private boolean containsAnyOperator(SearchParameters.FilterCriteria filter) {
323+
return containsAnyMatchingCriteria(filter, fc -> {
324+
if (fc.getOperator() == null) {
325+
return true;
326+
}
327+
return SearchParameters.Operator.fromString(fc.getOperator().toString()) == null;
328+
});
329+
}
330+
331+
private boolean containsAnyAttributeName(SearchParameters.FilterCriteria filter) {
332+
return containsAnyMatchingCriteria(filter, fc -> StringUtils.isBlank(fc.getAttributeName()));
333+
}
334+
335+
private boolean containsAnyAttributeValue(SearchParameters.FilterCriteria filter) {
336+
return containsAnyMatchingCriteria(filter, fc -> StringUtils.isBlank(fc.getAttributeValue()));
337+
}
338+
267339
@Test(dependsOnMethods = "testSavedSearch")
268340
public void testExecuteSavedSearchByName() {
269341
try {

‎webapp/src/test/resources/json/search-parameters/attribute-filters.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"offset": 0,
1010
"entityFilters": {
1111
"attributeName": "name",
12+
"operator": "contains",
1213
"attributeValue": "testtable",
1314
"condition" : "OR",
1415
"criterion" : [

‎webapp/src/test/resources/json/search-parameters/attribute-name.json‎

Lines changed: 156 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,157 @@
1-
[ {
2-
"testDescription": "hive_table contains testtable or retentionSize != 0",
3-
"searchParameters": {
4-
"typeName": "hive_table",
5-
"excludeDeletedEntities": true,
6-
"classification": "",
7-
"query": "",
8-
"limit": 25,
9-
"offset": 0,
10-
"entityFilters": {
11-
"attributeName": "",
12-
"attributeValue": "",
13-
"condition" : "AND",
14-
"criterion" : [
15-
{
16-
"attributeName": "",
17-
"operator": "eq",
18-
"attributeValue": "testtable"
19-
},
20-
{
21-
"attributeName": "retention",
22-
"operator": "neq",
23-
"attributeValue": "0"
24-
}
25-
]
26-
},
27-
"tagFilters": null,
28-
"attributes": [
29-
""
30-
]
1+
[
2+
{
3+
"testDescription": "Invalid attribute name is empty",
4+
"searchParameters": {
5+
"typeName": "hive_table",
6+
"excludeDeletedEntities": true,
7+
"limit": 25,
8+
"offset": 0,
9+
"entityFilters": {
10+
"condition": "AND",
11+
"criterion": [
12+
{
13+
"attributeName": "",
14+
"operator": "eq",
15+
"attributeValue": "testtable"
16+
},
17+
{
18+
"attributeName": "",
19+
"operator": "neq",
20+
"attributeValue": "0"
21+
}
22+
]
23+
},
24+
"tagFilters": null,
25+
"attributes": [""]
26+
}
3127
},
32-
"expectedCount": 0
33-
}
34-
]
28+
{
29+
"testDescription": "Nested invalid attribute names",
30+
"searchParameters": {
31+
"typeName": "hive_table",
32+
"excludeDeletedEntities": true,
33+
"limit": 25,
34+
"offset": 0,
35+
"entityFilters": {
36+
"condition": "AND",
37+
"criterion": [
38+
{
39+
"condition": "AND",
40+
"criterion": [
41+
{
42+
"attributeName": "",
43+
"operator": "eq",
44+
"attributeValue": "hive"
45+
},
46+
{
47+
"attributeName": "",
48+
"operator": "neq",
49+
"attributeValue": "hive"
50+
}
51+
]
52+
},
53+
{
54+
"attributeName": "",
55+
"operator": "neq",
56+
"attributeValue": ""
57+
}
58+
]
59+
},
60+
"tagFilters": null,
61+
"attributes": [""]
62+
}
63+
},
64+
{
65+
"testDescription": "Only nested tagFilters with invalid (empty) attribute names",
66+
"searchParameters": {
67+
"typeName": "hive_column",
68+
"excludeDeletedEntities": true,
69+
"limit": 25,
70+
"offset": 0,
71+
"entityFilters": null,
72+
"tagFilters": {
73+
"condition": "AND",
74+
"criterion": [
75+
{
76+
"condition": "OR",
77+
"criterion": [
78+
{
79+
"attributeName": "",
80+
"operator": "eq",
81+
"attributeValue": "admin"
82+
},
83+
{
84+
"attributeName": "",
85+
"operator": "neq",
86+
"attributeValue": "test_value"
87+
}
88+
]
89+
},
90+
{
91+
"attributeName": "",
92+
"operator": "eq",
93+
"attributeValue": "someValue"
94+
}
95+
]
96+
},
97+
"attributes": [""],
98+
"classification": "_ALL_CLASSIFICATION_TYPES"
99+
}
100+
},
101+
{
102+
"testDescription": "Nested entityFilters and tagFilters with invalid (empty) attribute names",
103+
"searchParameters": {
104+
"typeName": "hive_table",
105+
"excludeDeletedEntities": true,
106+
"limit": 25,
107+
"offset": 0,
108+
"entityFilters": {
109+
"condition": "AND",
110+
"criterion": [
111+
{
112+
"condition": "AND",
113+
"criterion": [
114+
{
115+
"attributeName": "",
116+
"operator": "eq",
117+
"attributeValue": "testtable"
118+
},
119+
{
120+
"attributeName": "",
121+
"operator": "eq",
122+
"attributeValue": "testuser"
123+
}
124+
]
125+
},
126+
{
127+
"attributeName": "",
128+
"operator": "neq",
129+
"attributeValue": "0"
130+
}
131+
]
132+
},
133+
"tagFilters": {
134+
"condition": "AND",
135+
"criterion": [
136+
{
137+
"condition": "OR",
138+
"criterion": [
139+
{
140+
"attributeName": "",
141+
"operator": "eq",
142+
"attributeValue": "v1"
143+
},
144+
{
145+
"attributeName": "",
146+
"operator": "eq",
147+
"attributeValue": "someValue"
148+
}
149+
]
150+
}
151+
]
152+
},
153+
"attributes": [""],
154+
"classification": "_ALL_CLASSIFICATION_TYPES"
155+
}
156+
}
157+
]

0 commit comments

Comments
 (0)